incubator-cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From m...@apache.org
Subject git commit: refs/heads/vm-snapshot - 1) remove snapshoting/reverting states from VM state machine, 2) change vmsnapshotservice interface
Date Fri, 01 Feb 2013 16:32:41 GMT
Updated Branches:
  refs/heads/vm-snapshot bb7cd90b6 -> ebca6890f


1) remove snapshoting/reverting states from VM state machine,2) change vmsnapshotservice interface


Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/ebca6890
Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/ebca6890
Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/ebca6890

Branch: refs/heads/vm-snapshot
Commit: ebca6890fd54f0ecc147f842b6c1366cf90957f9
Parents: bb7cd90
Author: Mice Xia <mice_xia@tcloudcomputing.com>
Authored: Sat Feb 2 00:36:21 2013 +0800
Committer: Mice Xia <mice_xia@tcloudcomputing.com>
Committed: Sat Feb 2 00:36:21 2013 +0800

----------------------------------------------------------------------
 api/src/com/cloud/event/EventTypes.java            |    2 +-
 api/src/com/cloud/vm/VirtualMachine.java           |   22 +--
 api/src/com/cloud/vm/snapshot/VMSnapshot.java      |    1 +
 .../com/cloud/vm/snapshot/VMSnapshotService.java   |   13 +-
 .../user/vmsnapshot/CreateVMSnapshotCmd.java       |    6 +-
 .../user/vmsnapshot/DeleteVMSnapshotCmd.java       |   22 +--
 .../command/user/vmsnapshot/ListVMSnapshotCmd.java |   12 -
 .../user/vmsnapshot/RevertToSnapshotCmd.java       |   20 +-
 .../com/cloud/vm/VirtualMachineManagerImpl.java    |   43 ++--
 .../com/cloud/vm/snapshot/VMSnapshotManager.java   |    7 +-
 .../cloud/vm/snapshot/VMSnapshotManagerImpl.java   |  184 ++++++++-------
 .../cloud/vm/snapshot/VMSnapshotManagerTest.java   |   60 +++---
 ui/scripts/instances.js                            |    8 -
 13 files changed, 173 insertions(+), 227 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/api/src/com/cloud/event/EventTypes.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/event/EventTypes.java b/api/src/com/cloud/event/EventTypes.java
index ffd0dd4..07ead20 100755
--- a/api/src/com/cloud/event/EventTypes.java
+++ b/api/src/com/cloud/event/EventTypes.java
@@ -295,7 +295,7 @@ public class EventTypes {
 	// vm snapshot events
     public static final String EVENT_VM_SNAPSHOT_CREATE = "VMSNAPSHOT.CREATE";
     public static final String EVENT_VM_SNAPSHOT_DELETE = "VMSNAPSHOT.DELETE";
-    public static final String EVENT_VM_SNAPSHOT_REVERT = "VMSNAPSHOT.REVERT";
+    public static final String EVENT_VM_SNAPSHOT_REVERT = "VMSNAPSHOT.REVERTTO";
 
     // external network device events
     public static final String EVENT_EXTERNAL_NVP_CONTROLLER_ADD = "PHYSICAL.NVPCONTROLLER.ADD";

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/api/src/com/cloud/vm/VirtualMachine.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/vm/VirtualMachine.java b/api/src/com/cloud/vm/VirtualMachine.java
index d7df268..358ab05 100755
--- a/api/src/com/cloud/vm/VirtualMachine.java
+++ b/api/src/com/cloud/vm/VirtualMachine.java
@@ -42,11 +42,7 @@ public interface VirtualMachine extends RunningOn, ControlledEntity, Identity, I
         Migrating(true, "VM is being migrated.  host id holds to from host"),
         Error(false, "VM is in error"),
         Unknown(false, "VM state is unknown."),
-        Shutdowned(false, "VM is shutdowned from inside"),
-        RunningSnapshotting(true, "VM is taking a snapshot in running state"),
-        StoppedSnapshotting(true, "VM is taking a snapshot in stopped state"),
-        RevertingToRunning(true, "VM is reverting to snapshot"),
-        RevertingToStopped(true, "VM is reverting to snapshot");
+        Shutdowned(false, "VM is shutdowned from inside");
 
         private final boolean _transitional;
         String _description;
@@ -114,22 +110,6 @@ public interface VirtualMachine extends RunningOn, ControlledEntity, Identity, I
             s_fsm.addTransition(State.Expunging, VirtualMachine.Event.ExpungeOperation, State.Expunging);
             s_fsm.addTransition(State.Error, VirtualMachine.Event.DestroyRequested, State.Expunging);
             s_fsm.addTransition(State.Error, VirtualMachine.Event.ExpungeOperation, State.Expunging);
-            
-            s_fsm.addTransition(State.Running, VirtualMachine.Event.SnapshotRequested, State.RunningSnapshotting);
-            s_fsm.addTransition(State.Stopped, VirtualMachine.Event.SnapshotRequested, State.StoppedSnapshotting);
-            s_fsm.addTransition(State.RunningSnapshotting, VirtualMachine.Event.OperationSucceeded, State.Running);
-            s_fsm.addTransition(State.StoppedSnapshotting, VirtualMachine.Event.OperationSucceeded, State.Stopped);
-            s_fsm.addTransition(State.RunningSnapshotting, VirtualMachine.Event.OperationFailed, State.Running);
-            s_fsm.addTransition(State.StoppedSnapshotting, VirtualMachine.Event.OperationFailed, State.Stopped);
-            
-            s_fsm.addTransition(State.Running, VirtualMachine.Event.RevertRequested, State.RevertingToRunning);
-            s_fsm.addTransition(State.Stopped, VirtualMachine.Event.RevertRequested, State.RevertingToStopped);
-            s_fsm.addTransition(State.RevertingToRunning, VirtualMachine.Event.OperationFailed, State.Running);
-            s_fsm.addTransition(State.RevertingToStopped, VirtualMachine.Event.OperationFailed, State.Stopped);
-            s_fsm.addTransition(State.RevertingToRunning, VirtualMachine.Event.OperationSucceeded, State.Running);
-            s_fsm.addTransition(State.RevertingToStopped, VirtualMachine.Event.OperationSucceeded, State.Stopped);
-
-            
         }
         
         public static boolean isVmStarted(State oldState, Event e, State newState) {

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/api/src/com/cloud/vm/snapshot/VMSnapshot.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/vm/snapshot/VMSnapshot.java b/api/src/com/cloud/vm/snapshot/VMSnapshot.java
index 7d08a2e..f0ee7ee 100644
--- a/api/src/com/cloud/vm/snapshot/VMSnapshot.java
+++ b/api/src/com/cloud/vm/snapshot/VMSnapshot.java
@@ -61,6 +61,7 @@ public interface VMSnapshot extends ControlledEntity, Identity, InternalIdentity
             s_fsm.addTransition(Reverting, Event.OperationFailed, Ready);
             s_fsm.addTransition(Ready, Event.ExpungeRequested, Expunging);
             s_fsm.addTransition(Error, Event.ExpungeRequested, Expunging);  
+            s_fsm.addTransition(Expunging, Event.ExpungeRequested, Expunging);  
             s_fsm.addTransition(Expunging, Event.OperationSucceeded, Removed);  
         }
     }

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/api/src/com/cloud/vm/snapshot/VMSnapshotService.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/vm/snapshot/VMSnapshotService.java b/api/src/com/cloud/vm/snapshot/VMSnapshotService.java
index a1909fa..83f86bc 100644
--- a/api/src/com/cloud/vm/snapshot/VMSnapshotService.java
+++ b/api/src/com/cloud/vm/snapshot/VMSnapshotService.java
@@ -19,10 +19,7 @@ package com.cloud.vm.snapshot;
 
 import java.util.List;
 
-import org.apache.cloudstack.api.command.user.vmsnapshot.CreateVMSnapshotCmd;
-import org.apache.cloudstack.api.command.user.vmsnapshot.DeleteVMSnapshotCmd;
 import org.apache.cloudstack.api.command.user.vmsnapshot.ListVMSnapshotCmd;
-import org.apache.cloudstack.api.command.user.vmsnapshot.RevertToSnapshotCmd;
 
 import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.exception.InsufficientCapacityException;
@@ -36,16 +33,16 @@ public interface VMSnapshotService {
 
     List<? extends VMSnapshot> listVMSnapshots(ListVMSnapshotCmd cmd);
 
-    VMSnapshot getVMSnapshotById(long id);
+    VMSnapshot getVMSnapshotById(Long id);
 
-    VMSnapshot creatVMSnapshot(CreateVMSnapshotCmd cmd);
+    VMSnapshot creatVMSnapshot(Long vmId, Long vmSnapshotId);
 
-    VMSnapshot allocVMSnapshot(CreateVMSnapshotCmd cmd)
+    VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDescription, Boolean snapshotMemory)
             throws ResourceAllocationException;
 
-    boolean deleteVMSnapshot(DeleteVMSnapshotCmd cmd);
+    boolean deleteVMSnapshot(Long vmSnapshotId);
 
-    UserVm revertToSnapshot(RevertToSnapshotCmd cmd) throws InsufficientServerCapacityException, InsufficientCapacityException, ResourceUnavailableException, ConcurrentOperationException;
+    UserVm revertToSnapshot(Long vmSnapshotId) throws InsufficientServerCapacityException, InsufficientCapacityException, ResourceUnavailableException, ConcurrentOperationException;
 
     VirtualMachine getVMBySnapshotId(Long id);
 }

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java b/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java
index 7ded716..45cc9f1 100644
--- a/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java
@@ -75,7 +75,7 @@ public class CreateVMSnapshotCmd extends BaseAsyncCreateCmd {
 
     @Override
     public void create() throws ResourceAllocationException {
-        VMSnapshot vmsnapshot = _vmSnapshotService.allocVMSnapshot(this);
+        VMSnapshot vmsnapshot = _vmSnapshotService.allocVMSnapshot(getVmId(),getDisplayName(),getDescription(),snapshotMemory());
         if (vmsnapshot != null) {
             this.setEntityId(vmsnapshot.getId());
         } else {
@@ -86,7 +86,7 @@ public class CreateVMSnapshotCmd extends BaseAsyncCreateCmd {
 
     @Override
     public String getEventDescription() {
-        return "creating snapshot for vm: " + getVmId();
+        return "creating snapshot for VM: " + getVmId();
     }
 
     @Override
@@ -97,7 +97,7 @@ public class CreateVMSnapshotCmd extends BaseAsyncCreateCmd {
     @Override
     public void execute() {
         UserContext.current().setEventDetails("VM Id: " + getVmId());
-        VMSnapshot result = _vmSnapshotService.creatVMSnapshot(this);
+        VMSnapshot result = _vmSnapshotService.creatVMSnapshot(getVmId(),getEntityId());
         if (result != null) {
             VMSnapshotResponse response = _responseGenerator
                     .createVMSnapshotResponse(result);

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/DeleteVMSnapshotCmd.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/DeleteVMSnapshotCmd.java b/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/DeleteVMSnapshotCmd.java
index 592bc66..a2b2c08 100644
--- a/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/DeleteVMSnapshotCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/DeleteVMSnapshotCmd.java
@@ -17,30 +17,20 @@
 
 package org.apache.cloudstack.api.command.user.vmsnapshot;
 
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.BaseAsyncCmd;
-import org.apache.cloudstack.api.BaseCmd;
-import org.apache.cloudstack.api.BaseCmd.CommandType;
-import org.apache.cloudstack.api.response.SuccessResponse;
-import org.apache.cloudstack.api.response.VolumeResponse;
 import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.ServerApiException;
-
+import org.apache.cloudstack.api.response.SuccessResponse;
 import org.apache.cloudstack.api.response.VMSnapshotResponse;
+import org.apache.log4j.Logger;
+
 import com.cloud.event.EventTypes;
-import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.storage.Snapshot;
-import com.cloud.storage.Volume;
 import com.cloud.user.Account;
 import com.cloud.user.UserContext;
-import com.cloud.uservm.UserVm;
-import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.snapshot.VMSnapshot;
-//import com.cloud.api.ApiDBUtils;
 
 @APICommand(name="deleteVMSnapshot", description = "Deletes a vmsnapshot.", responseObject = SuccessResponse.class)
 public class DeleteVMSnapshotCmd extends BaseAsyncCmd {
@@ -61,10 +51,6 @@ public class DeleteVMSnapshotCmd extends BaseAsyncCmd {
         return s_name;
     }
 
-    public static String getResultObjectName() {
-        return "vm_snapshots";
-    }
-
     @Override
     public long getEntityOwnerId() {
         VMSnapshot vmSnapshot = _entityMgr.findById(VMSnapshot.class, getId());
@@ -77,7 +63,7 @@ public class DeleteVMSnapshotCmd extends BaseAsyncCmd {
     @Override
     public void execute() {
         UserContext.current().setEventDetails("vmsnapshot id: " + getId());
-        boolean result = _vmSnapshotService.deleteVMSnapshot(this);
+        boolean result = _vmSnapshotService.deleteVMSnapshot(getId());
         if (result) {
             SuccessResponse response = new SuccessResponse(getCommandName());
             this.setResponseObject(response);

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/ListVMSnapshotCmd.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/ListVMSnapshotCmd.java b/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/ListVMSnapshotCmd.java
index 8b18484..3c7052d 100644
--- a/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/ListVMSnapshotCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/ListVMSnapshotCmd.java
@@ -39,10 +39,6 @@ public class ListVMSnapshotCmd extends BaseListTaggedResourcesCmd {
 
     private static final String s_name = "listvmsnapshotresponse";
 
-    // ///////////////////////////////////////////////////
-    // ////////////// API parameters /////////////////////
-    // ///////////////////////////////////////////////////
-
     @Parameter(name=ApiConstants.VM_SNAPSHOT_ID, type=CommandType.UUID, entityType=VMSnapshotResponse.class,
              description="The ID of the VM snapshot")
     private Long id;
@@ -56,10 +52,6 @@ public class ListVMSnapshotCmd extends BaseListTaggedResourcesCmd {
     @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "lists snapshot by snapshot name or display name")
     private String vmSnapshotName;
 
-    // ///////////////////////////////////////////////////
-    // ///////////////// Accessors ///////////////////////
-    // ///////////////////////////////////////////////////
-
     public String getState() {
         return state;
     }
@@ -76,10 +68,6 @@ public class ListVMSnapshotCmd extends BaseListTaggedResourcesCmd {
         return id;
     }
 
-    // ///////////////////////////////////////////////////
-    // ///////////// API Implementation///////////////////
-    // ///////////////////////////////////////////////////
-
     @Override
     public void execute() {
         List<? extends VMSnapshot> result = _vmSnapshotService

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/RevertToSnapshotCmd.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/RevertToSnapshotCmd.java b/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/RevertToSnapshotCmd.java
index 6642a85..d7b4599 100644
--- a/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/RevertToSnapshotCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/vmsnapshot/RevertToSnapshotCmd.java
@@ -30,9 +30,9 @@ import org.apache.cloudstack.api.response.VMSnapshotResponse;
 import com.cloud.event.EventTypes;
 import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.exception.InsufficientCapacityException;
-import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.exception.ResourceAllocationException;
 import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.user.Account;
 import com.cloud.user.UserContext;
 import com.cloud.uservm.UserVm;
 import com.cloud.vm.snapshot.VMSnapshot;
@@ -55,33 +55,27 @@ public class RevertToSnapshotCmd extends BaseAsyncCmd {
         return s_name;
     }
 
-    public static String getResultObjectName() {
-        return "vm_snapshots";
-    }
-
     @Override
     public long getEntityOwnerId() {
-        VMSnapshot vmSnapshot = _vmSnapshotService.getVMSnapshotById(getVmSnapShotId());
-        if (vmSnapshot == null) {
-            throw new InvalidParameterValueException(
-                    "Unable to find the snapshot by id=" + getVmSnapShotId());
+        VMSnapshot vmSnapshot = _entityMgr.findById(VMSnapshot.class, getVmSnapShotId());
+        if (vmSnapshot != null) {
+            return vmSnapshot.getAccountId();
         }
-        UserVm userVM = _userVmService.getUserVm(vmSnapshot.getVmId());
-        return userVM.getAccountId();
+        return Account.ACCOUNT_ID_SYSTEM;
     }
 
     @Override
     public void execute() throws  ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException, ConcurrentOperationException {
         UserContext.current().setEventDetails(
                 "vmsnapshot id: " + getVmSnapShotId());
-        UserVm result = _vmSnapshotService.revertToSnapshot(this);
+        UserVm result = _vmSnapshotService.revertToSnapshot(getVmSnapShotId());
         if (result != null) {
             UserVmResponse response = _responseGenerator.createUserVmResponse(
                     "virtualmachine", result).get(0);
             response.setResponseName(getCommandName());
             this.setResponseObject(response);
         } else {
-            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,"Failed to revert vm snapshot");
+            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR,"Failed to revert VM snapshot");
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
index a5b7129..8dd63e0 100755
--- a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
+++ b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -154,7 +154,6 @@ import com.cloud.utils.fsm.StateMachine2;
 import com.cloud.vm.ItWorkVO.Step;
 import com.cloud.vm.VirtualMachine.Event;
 import com.cloud.vm.VirtualMachine.State;
-import com.cloud.vm.VirtualMachineProfile.Param;
 import com.cloud.vm.dao.ConsoleProxyDao;
 import com.cloud.vm.dao.DomainRouterDao;
 import com.cloud.vm.dao.NicDao;
@@ -1162,6 +1161,12 @@ public class VirtualMachineManagerImpl implements VirtualMachineManager, Listene
 
     @Override
     public boolean stateTransitTo(VMInstanceVO vm, VirtualMachine.Event e, Long hostId) throws NoTransitionException {
+        // if there are active vm snapshots task, state change is not allowed
+        if(_vmSnapshotMgr.hasActiveVMSnapshotTasks(vm.getId())){
+            s_logger.error("State transit with event: " + e + " failed due to: " + vm.getInstanceName() + " has active VM snapshots tasks");
+            return false;
+        }
+        
         State oldState = vm.getState();
         if (oldState == State.Starting) {
             if (e == Event.OperationSucceeded) {
@@ -1650,19 +1655,15 @@ public class VirtualMachineManagerImpl implements VirtualMachineManager, Listene
             AgentVmInfo info = infos.remove(vm.getId());
             
             // sync VM Snapshots related transient states
-            List<VMSnapshotVO> expungingVMSnapshot = _vmSnapshotDao.listByInstanceId(vm.getId(), VMSnapshot.State.Expunging);
-            if( vm.getState() == State.RevertingToRunning || vm.getState() == State.RevertingToStopped 
-                    || vm.getState() == State.RunningSnapshotting || vm.getState() == State.StoppedSnapshotting 
-                    || expungingVMSnapshot.size() == 1){
-                s_logger.info("Found vm " + vm.getInstanceName() + " in state. " + vm.getState() + ", needs to sync VM snapshot state");
-                if(!_vmSnapshotMgr.syncVMSnapshot(vm, null, hostId)){
+            List<VMSnapshotVO> vmSnapshotsInTrasientStates = _vmSnapshotDao.listByInstanceId(vm.getId(), VMSnapshot.State.Expunging,VMSnapshot.State.Reverting, VMSnapshot.State.Creating);
+            if(vmSnapshotsInTrasientStates.size() > 1){
+                s_logger.info("Found vm " + vm.getInstanceName() + " with VM snapshots in transient states, needs to sync VM snapshot state");
+                if(!_vmSnapshotMgr.syncVMSnapshot(vm, hostId)){
                     s_logger.warn("Failed to sync VM in a transient snapshot related state: " + vm.getInstanceName());
                     continue;
                 }else{
-                    s_logger.info("Successfully sync VM in a transient snapshot related state: " + vm.getInstanceName() + " to " + vm.getState());
+                    s_logger.info("Successfully sync VM with transient snapshot: " + vm.getInstanceName());
                 }
-                if(expungingVMSnapshot.size() == 1)
-                    _vmSnapshotMgr.syncVMSnapshot(vm, expungingVMSnapshot.get(0), hostId);
             }
             
             VMInstanceVO castedVm = null;
@@ -1784,29 +1785,25 @@ public class VirtualMachineManagerImpl implements VirtualMachineManager, Listene
             VMInstanceVO castedVm = null;
             
             // sync VM Snapshots related transient states
-            List<VMSnapshotVO> expungingVMSnapshot = _vmSnapshotDao.listByInstanceId(vm.getId(), VMSnapshot.State.Expunging);
-            if( vm.getState() == State.RevertingToRunning || vm.getState() == State.RevertingToStopped 
-                    || vm.getState() == State.RunningSnapshotting || vm.getState() == State.StoppedSnapshotting 
-                    || expungingVMSnapshot.size() == 1){
+            List<VMSnapshotVO> vmSnapshotsInExpungingStates = _vmSnapshotDao.listByInstanceId(vm.getId(), VMSnapshot.State.Expunging, VMSnapshot.State.Creating,VMSnapshot.State.Reverting);
+            if(vmSnapshotsInExpungingStates.size() > 0){
                 s_logger.info("Found vm " + vm.getInstanceName() + " in state. " + vm.getState() + ", needs to sync VM snapshot state");
                 Long hostId = null;
+                Host host = null;
                 if(info != null && info.getHostUuid() != null){
-                    Host host = _hostDao.findByGuid(info.getHostUuid());
-                    hostId = host == null ? (vm.getHostId() == null ? vm.getLastHostId() : vm.getHostId()) : host.getId();
+                    host = _hostDao.findByGuid(info.getHostUuid());
                 }
-                if(!_vmSnapshotMgr.syncVMSnapshot(vm, null, hostId)){
-                    s_logger.warn("Failed to sync VM in a transient snapshot related state: " + vm.getInstanceName());
+                hostId = host == null ? (vm.getHostId() == null ? vm.getLastHostId() : vm.getHostId()) : host.getId();
+                if(!_vmSnapshotMgr.syncVMSnapshot(vm, hostId)){
+                    s_logger.warn("Failed to sync VM with transient snapshot: " + vm.getInstanceName());
                     continue;
                 }else{
-                    s_logger.info("Successfully sync VM in a transient snapshot related state: " + vm.getInstanceName() + " to " + vm.getState());
+                    s_logger.info("Successfully sync VM with transient snapshot: " + vm.getInstanceName());
                 }
-                if(expungingVMSnapshot.size() == 1)
-                    _vmSnapshotMgr.syncVMSnapshot(vm, expungingVMSnapshot.get(0), hostId);
             }
             
             if ((info == null && (vm.getState() == State.Running || vm.getState() == State.Starting ))  
-            		||  (info != null && (info.state == State.Running && vm.getState() == State.Starting))
-					||  (info != null && (info.state == State.Running && vm.getState() == State.RevertingToRunning))) 
+            		||  (info != null && (info.state == State.Running && vm.getState() == State.Starting))) 
             {
             	s_logger.info("Found vm " + vm.getInstanceName() + " in inconsistent state. " + vm.getState() + " on CS while " +  (info == null ? "Stopped" : "Running") + " on agent");
                 info = new AgentVmInfo(vm.getInstanceName(), getVmGuru(vm), vm, State.Stopped);

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/server/src/com/cloud/vm/snapshot/VMSnapshotManager.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/snapshot/VMSnapshotManager.java b/server/src/com/cloud/vm/snapshot/VMSnapshotManager.java
index 77c3b69..c609005 100644
--- a/server/src/com/cloud/vm/snapshot/VMSnapshotManager.java
+++ b/server/src/com/cloud/vm/snapshot/VMSnapshotManager.java
@@ -33,14 +33,15 @@ public interface VMSnapshotManager extends VMSnapshotService, Manager {
     boolean deleteAllVMSnapshots(long id, VMSnapshot.Type type);
 
     /**
-     * Sync VM's state when VM in reverting or snapshotting, or VM snapshot in expunging state
+     * Sync VM snapshot state when VM snapshot in reverting or snapshoting or expunging state
      * Used for fullsync after agent connects
      * 
      * @param vm, the VM in question
-     * @param vmSnapshot, if this is not null, sync an VM snapshot in expunging state
      * @param hostId
      * @return true if succeeds, false if fails
      */
-    boolean syncVMSnapshot(VMInstanceVO vm, VMSnapshotVO vmSnapshot, Long hostId);
+    boolean syncVMSnapshot(VMInstanceVO vm, Long hostId);
+
+    boolean hasActiveVMSnapshotTasks(Long vmId);
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
index 49a4eec..7541e52 100644
--- a/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
+++ b/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@ -26,10 +26,7 @@ import java.util.Map;
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
 
-import org.apache.cloudstack.api.command.user.vmsnapshot.CreateVMSnapshotCmd;
-import org.apache.cloudstack.api.command.user.vmsnapshot.DeleteVMSnapshotCmd;
 import org.apache.cloudstack.api.command.user.vmsnapshot.ListVMSnapshotCmd;
-import org.apache.cloudstack.api.command.user.vmsnapshot.RevertToSnapshotCmd;
 import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
@@ -59,7 +56,6 @@ import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.hypervisor.HypervisorGuruManager;
 import com.cloud.projects.Project.ListProjectResourcesCriteria;
-import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.storage.GuestOSVO;
 import com.cloud.storage.Snapshot;
 import com.cloud.storage.SnapshotVO;
@@ -78,7 +74,6 @@ import com.cloud.user.dao.UserDao;
 import com.cloud.uservm.UserVm;
 import com.cloud.utils.DateUtil;
 import com.cloud.utils.NumbersUtil;
-import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;
 import com.cloud.utils.component.ComponentLocator;
 import com.cloud.utils.component.Inject;
@@ -121,7 +116,6 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
     @Inject VirtualMachineManager _itMgr;
     @Inject ConfigurationDao _configDao;
     int _vmSnapshotMax;
-    StateMachine2<State, VirtualMachine.Event, VirtualMachine> _stateMachine;
     StateMachine2<VMSnapshot.State, VMSnapshot.Event, VMSnapshot> _vmSnapshottateMachine ;
 
     @Override
@@ -133,11 +127,10 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
         if (_configDao == null) {
             throw new ConfigurationException("Unable to get the configuration dao.");
         }
-        s_logger.info("Snapshot Manager is configured.");
+        s_logger.info("VM Snapshot Manager is configured.");
 
         _vmSnapshotMax = NumbersUtil.parseInt(_configDao.getValue("vmsnapshot.max"), VMSNAPSHOTMAX);
 
-        _stateMachine = VirtualMachine.State.getStateMachine();
         _vmSnapshottateMachine   = VMSnapshot.State.getStateMachine();
         return true;
     }
@@ -236,11 +229,8 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
     }
     
     @Override
-    public VMSnapshot allocVMSnapshot(CreateVMSnapshotCmd cmd) throws ResourceAllocationException {
-        Long vmId = cmd.getVmId();
-        String vsDisplayName = cmd.getDisplayName();
-        String vsDescription = cmd.getDescription();
-        Boolean snapshotMemory = cmd.snapshotMemory();
+    public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDescription, Boolean snapshotMemory) 
+            throws ResourceAllocationException {
 
         Account caller = getCaller();
 
@@ -250,6 +240,12 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
             throw new InvalidParameterValueException("Creating VM snapshot failed due to VM:" + vmId + " is a system VM or does not exist");
         }
         
+        // parameter length check
+        if(vsDisplayName != null && vsDisplayName.length()>255)
+            throw new InvalidParameterValueException("Creating VM snapshot failed due to length of VM snapshot vsDisplayName should not exceed 255");
+        if(vsDescription != null && vsDescription.length()>255)
+            throw new InvalidParameterValueException("Creating VM snapshot failed due to length of VM snapshot vsDescription should not exceed 255");
+        
         // VM snapshot display name must be unique for a VM
         String timeString = DateUtil.getDateDisplayString(DateUtil.GMT_TIMEZONE, new Date(), DateUtil.YYYYMMDD_FORMAT);
         String vmSnapshotName = userVmVo.getInstanceName() + "_VS_" + timeString;
@@ -265,6 +261,10 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
             throw new InvalidParameterValueException("Creating vm snapshot failed due to VM:" + vmId + " is not in the running or Stopped state");
         }
         
+        if(snapshotMemory && userVmVo.getState() == VirtualMachine.State.Stopped){
+            throw new InvalidParameterValueException("Can not snapshot memory when VM is in stopped state");
+        }
+        
         // for KVM, only allow snapshot with memory when VM is in running state
         if(userVmVo.getHypervisorType() == HypervisorType.KVM && userVmVo.getState() == State.Running && !snapshotMemory){
             throw new InvalidParameterValueException("KVM VM does not allow to take a disk-only snapshot when VM is in running state");
@@ -291,7 +291,7 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
         }
         
         // check if there are other active VM snapshot tasks
-        if (checkActiveVMSnapshotTasks(vmId)) {
+        if (hasActiveVMSnapshotTasks(vmId)) {
             throw new CloudRuntimeException("There is other active vm snapshot tasks on the instance, please try again later");
         }
         
@@ -320,13 +320,22 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
     }
 
     @Override
-    @ActionEvent(eventType = EventTypes.EVENT_VM_SNAPSHOT_CREATE, eventDescription = "creating_vm_snapshot", async = true)
-    public VMSnapshot creatVMSnapshot(CreateVMSnapshotCmd cmd) {
-        Long vmId = cmd.getVmId();
+    @ActionEvent(eventType = EventTypes.EVENT_VM_SNAPSHOT_CREATE, eventDescription = "creating VM snapshot", async = true)
+    public VMSnapshot creatVMSnapshot(Long vmId, Long vmSnapshotId) {
         UserVmVO userVm = _userVMDao.findById(vmId);
-        VMSnapshotVO vmSnapshot = _vmSnapshotDao.findById(cmd.getEntityId());
+        if (userVm == null) {
+            throw new InvalidParameterValueException("Create vm to snapshot failed due to vm: " + vmId + " is not found");
+        }
+        VMSnapshotVO vmSnapshot = _vmSnapshotDao.findById(vmSnapshotId);
+        if(vmSnapshot == null){
+            throw new CloudRuntimeException("VM snapshot id: " + vmSnapshotId + " can not be found");
+        }
         Long hostId = pickRunningHost(vmId);
-        transitState(vmSnapshot, VMSnapshot.Event.CreateRequested, userVm,  VirtualMachine.Event.SnapshotRequested, hostId);
+        try {
+            vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.CreateRequested);
+        } catch (NoTransitionException e) {
+            throw new CloudRuntimeException(e.getMessage());
+        }
         return createVmSnapshotInternal(userVm, vmSnapshot, hostId);
     }
 
@@ -355,15 +364,21 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
             answer = (CreateVMSnapshotAnswer) sendToPool(hostId, ccmd);
             if (answer != null && answer.getResult()) {
                 processAnswer(vmSnapshot, userVm, answer, hostId);
-                transitState(vmSnapshot, VMSnapshot.Event.OperationSucceeded, userVm, VirtualMachine.Event.OperationSucceeded, hostId);
                 s_logger.debug("Create vm snapshot " + vmSnapshot.getName() + " succeeded for vm: " + userVm.getInstanceName());
             }else{
                 String errMsg = answer.getDetails();
                 s_logger.error("Agent reports creating vm snapshot " + vmSnapshot.getName() + " failed for vm: " + userVm.getInstanceName() + " due to " + errMsg);
-                transitState(vmSnapshot, VMSnapshot.Event.OperationFailed, userVm,  VirtualMachine.Event.OperationFailed, hostId);
+                vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed);
             }
             return vmSnapshot;
         } catch (Exception e) {
+            if(e instanceof AgentUnavailableException){
+                try {
+                    vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed);
+                } catch (NoTransitionException e1) {
+                    s_logger.error("Cannot set vm snapshot state due to: " + e1.getMessage());
+                }
+            }
             String msg = e.getMessage();
             s_logger.error("Create vm snapshot " + vmSnapshot.getName() + " failed for vm: " + userVm.getInstanceName() + " due to " + msg);
             throw new CloudRuntimeException(msg);
@@ -412,27 +427,11 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
                 vo.getCurrent(), null);
     }
 
-    private boolean vmSnapshotStateTransitTo(VMSnapshotVO vsnp, VMSnapshot.Event event) throws NoTransitionException {
+    protected boolean vmSnapshotStateTransitTo(VMSnapshotVO vsnp, VMSnapshot.Event event) throws NoTransitionException {
         return _vmSnapshottateMachine.transitTo(vsnp, event, null, _vmSnapshotDao);
     }
     
     @DB
-    protected boolean transitState(VMSnapshotVO vsnp, VMSnapshot.Event e1, VMInstanceVO vm, VirtualMachine.Event e2, Long hostId){
-        final Transaction txn = Transaction.currentTxn();
-        try {
-            txn.start();
-            vmSnapshotStateTransitTo(vsnp, e1);
-            vmStateTransitTo(vm, e2, hostId, null);
-            return txn.commit();
-        } catch (NoTransitionException e) {
-            txn.rollback();
-            return false;
-        }finally{
-            txn.close();
-        }
-    }
-    
-    @DB
     protected void processAnswer(VMSnapshotVO vmSnapshot, UserVmVO  userVm, Answer as, Long hostId) {
         final Transaction txn = Transaction.currentTxn();
         try {
@@ -440,9 +439,11 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
             if (as instanceof CreateVMSnapshotAnswer) {
                 CreateVMSnapshotAnswer answer = (CreateVMSnapshotAnswer) as;
                 finalizeCreate(vmSnapshot, answer.getVolumeTOs());
+                vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded);
             } else if (as instanceof RevertToVMSnapshotAnswer) {
                 RevertToVMSnapshotAnswer answer = (RevertToVMSnapshotAnswer) as;
                 finalizeRevert(vmSnapshot, answer.getVolumeTOs());
+                vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded);
             } else if (as instanceof DeleteVMSnapshotAnswer) {
                 DeleteVMSnapshotAnswer answer = (DeleteVMSnapshotAnswer) as;
                 finalizeDelete(vmSnapshot, answer.getVolumeTOs());
@@ -502,9 +503,11 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
 
         // update current snapshot, current snapshot is the one reverted to
         VMSnapshotVO previousCurrent = _vmSnapshotDao.findCurrentSnapshotByVmId(vmSnapshot.getVmId());
-        previousCurrent.setCurrent(false);
+        if(previousCurrent != null){
+            previousCurrent.setCurrent(false);
+            _vmSnapshotDao.persist(previousCurrent);
+        }
         vmSnapshot.setCurrent(true);
-        _vmSnapshotDao.persist(previousCurrent);
         _vmSnapshotDao.persist(vmSnapshot);
     }
 
@@ -528,16 +531,16 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
         return answer;
     }
     
-    private boolean checkActiveVMSnapshotTasks(Long vmId){
+    @Override
+    public boolean hasActiveVMSnapshotTasks(Long vmId){
         List<VMSnapshotVO> activeVMSnapshots = _vmSnapshotDao.listByInstanceId(vmId, 
                 VMSnapshot.State.Creating, VMSnapshot.State.Expunging,VMSnapshot.State.Reverting,VMSnapshot.State.Allocated);
         return activeVMSnapshots.size() > 0;
     }
 
     @Override
-    @ActionEvent(eventType = EventTypes.EVENT_VM_SNAPSHOT_DELETE, eventDescription = "delete_vm_snapshot", async=true)
-    public boolean deleteVMSnapshot(DeleteVMSnapshotCmd cmd) {
-        Long vmSnapshotId = cmd.getId();
+    @ActionEvent(eventType = EventTypes.EVENT_VM_SNAPSHOT_DELETE, eventDescription = "delete vm snapshots", async=true)
+    public boolean deleteVMSnapshot(Long vmSnapshotId) {
         Account caller = getCaller();
 
         VMSnapshotVO vmSnapshot = _vmSnapshotDao.findById(vmSnapshotId);
@@ -548,13 +551,17 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
         _accountMgr.checkAccess(caller, null, true, vmSnapshot);
         
         // check VM snapshot states, only allow to delete vm snapshots in created and error state
-        if (VMSnapshot.State.Ready != vmSnapshot.getState() && VMSnapshot.State.Error != vmSnapshot.getState()) {
-            throw new InvalidParameterValueException("Can't delete the vm snapshotshot " + vmSnapshotId + " due to it is not in Created or Error State");
+        if (VMSnapshot.State.Ready != vmSnapshot.getState() && VMSnapshot.State.Expunging != vmSnapshot.getState() && VMSnapshot.State.Error != vmSnapshot.getState()) {
+            throw new InvalidParameterValueException("Can't delete the vm snapshotshot " + vmSnapshotId + " due to it is not in Created or Error, or Expunging State");
         }
         
         // check if there are other active VM snapshot tasks
-        if (checkActiveVMSnapshotTasks(vmSnapshot.getVmId())) {
-            throw new InvalidParameterValueException("There is other active vm snapshot tasks on the instance, please try again later");
+        if (hasActiveVMSnapshotTasks(vmSnapshot.getVmId())) {
+            List<VMSnapshotVO> expungingSnapshots = _vmSnapshotDao.listByInstanceId(vmSnapshot.getVmId(), VMSnapshot.State.Expunging);
+            if(expungingSnapshots.size() > 0 && expungingSnapshots.get(0).getId() == vmSnapshot.getId())
+                s_logger.debug("Target VM snapshot already in expunging state, go on deleting it: " + vmSnapshot.getDisplayName());
+            else
+                throw new InvalidParameterValueException("There is other active vm snapshot tasks on the instance, please try again later");
         }
 
         if(vmSnapshot.getState() == VMSnapshot.State.Allocated){
@@ -587,6 +594,7 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
            
             if (answer != null && answer.getResult()) {
                 processAnswer(vmSnapshot, userVm, answer, hostId);
+                s_logger.debug("Delete VM snapshot " + vmSnapshot.getName() + " succeeded for vm: " + userVm.getInstanceName());
                 return true;
             } else {
                 s_logger.error("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + answer.getDetails());
@@ -600,11 +608,9 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
     }
 
     @Override
-    @ActionEvent(eventType = EventTypes.EVENT_VM_SNAPSHOT_REVERT, eventDescription = "revert_vm", async = true)
-    public UserVm revertToSnapshot(RevertToSnapshotCmd cmd) throws InsufficientCapacityException, ResourceUnavailableException, ConcurrentOperationException {
+    @ActionEvent(eventType = EventTypes.EVENT_VM_SNAPSHOT_REVERT, eventDescription = "revert to VM snapshot", async = true)
+    public UserVm revertToSnapshot(Long vmSnapshotId) throws InsufficientCapacityException, ResourceUnavailableException, ConcurrentOperationException {
 
-        Long vmSnapshotId = cmd.getVmSnapShotId();
-        
         // check if VM snapshot exists in DB
         VMSnapshotVO vmSnapshotVo = _vmSnapshotDao.findById(vmSnapshotId);
         if (vmSnapshotVo == null) {
@@ -621,7 +627,7 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
         }
         
         // check if there are other active VM snapshot tasks
-        if (checkActiveVMSnapshotTasks(vmId)) {
+        if (hasActiveVMSnapshotTasks(vmId)) {
             throw new InvalidParameterValueException("There is other active vm snapshot tasks on the instance, please try again later");
         }
         
@@ -672,19 +678,21 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
             throw new CloudRuntimeException("Can not find any host to revert snapshot " + vmSnapshotVo.getName());
         
         // check if there are other active VM snapshot tasks
-        if (checkActiveVMSnapshotTasks(userVm.getId())) {
+        if (hasActiveVMSnapshotTasks(userVm.getId())) {
             throw new InvalidParameterValueException("There is other active vm snapshot tasks on the instance, please try again later");
         }
         
         userVm = _userVMDao.findById(userVm.getId());
-        transitState(vmSnapshotVo, VMSnapshot.Event.RevertRequested, userVm, VirtualMachine.Event.RevertRequested, hostId);
+        try {
+            vmSnapshotStateTransitTo(vmSnapshotVo, VMSnapshot.Event.RevertRequested);
+        } catch (NoTransitionException e) {
+            throw new CloudRuntimeException(e.getMessage());
+        }
         return revertInternal(userVm, vmSnapshotVo, hostId);
     }
 
     private UserVm revertInternal(UserVmVO userVm, VMSnapshotVO vmSnapshotVo, Long hostId) {
-
         try {
-            
             VMSnapshotVO snapshot = _vmSnapshotDao.findById(vmSnapshotVo.getId());
             // prepare RevertToSnapshotCommand
             List<VolumeTO> volumeTOs = getVolumeTOList(userVm.getId());
@@ -697,20 +705,27 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
             RevertToVMSnapshotCommand revertToSnapshotCommand = new RevertToVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs, guestOS.getDisplayName());
            
             RevertToVMSnapshotAnswer answer = (RevertToVMSnapshotAnswer) sendToPool(hostId, revertToSnapshotCommand);
-            if (answer.getResult()) {
+            if (answer != null && answer.getResult()) {
                 processAnswer(vmSnapshotVo, userVm, answer, hostId);
-                transitState(vmSnapshotVo, VMSnapshot.Event.OperationSucceeded, userVm, VirtualMachine.Event.OperationSucceeded, hostId);
+                s_logger.debug("RevertTo " + vmSnapshotVo.getName() + " succeeded for vm: " + userVm.getInstanceName());
             } else {
                 String errMsg = "Revert VM: " + userVm.getInstanceName() + " to snapshot: "+ vmSnapshotVo.getName() + " failed";
                 if(answer != null && answer.getDetails() != null)
                     errMsg = errMsg + " due to " + answer.getDetails();
                 s_logger.error(errMsg);
                 // agent report revert operation fails
-                transitState(vmSnapshotVo, VMSnapshot.Event.OperationFailed, userVm, VirtualMachine.Event.OperationFailed, hostId);
+                vmSnapshotStateTransitTo(vmSnapshotVo, VMSnapshot.Event.OperationFailed);
                 throw new CloudRuntimeException(errMsg);
             }
         } catch (Exception e) {
-            // for other exceptions, do not change VM state, leave it for snapshotSync
+            if(e instanceof AgentUnavailableException){
+                try {
+                    vmSnapshotStateTransitTo(vmSnapshotVo, VMSnapshot.Event.OperationFailed);
+                } catch (NoTransitionException e1) {
+                    s_logger.error("Cannot set vm snapshot state due to: " + e1.getMessage());
+                }
+            }
+            // for other exceptions, do not change VM snapshot state, leave it for snapshotSync
             String errMsg = "revert vm: " + userVm.getInstanceName() + " to snapshot " + vmSnapshotVo.getName() + " failed due to " + e.getMessage();
             s_logger.error(errMsg);
             throw new CloudRuntimeException(e.getMessage());
@@ -718,18 +733,14 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
         return userVm;
     }
 
-    private boolean vmStateTransitTo(VMInstanceVO vm, VirtualMachine.Event e, Long hostId, String reservationId) throws NoTransitionException {
-        vm.setReservationId(reservationId);
-        return _stateMachine.transitTo(vm, e, new Pair<Long, Long>(vm.getHostId(), hostId), _vmInstanceDao);
-    }
 
     @Override
-    public VMSnapshot getVMSnapshotById(long id) {
+    public VMSnapshot getVMSnapshotById(Long id) {
         VMSnapshotVO vmSnapshot = _vmSnapshotDao.findById(id);
         return vmSnapshot;
     }
 
-    private Long pickRunningHost(Long vmId) {
+    protected Long pickRunningHost(Long vmId) {
         UserVmVO vm = _userVMDao.findById(vmId);
         // use VM's host if VM is running
         if(vm.getState() == State.Running)
@@ -766,6 +777,9 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
     @Override
     public VirtualMachine getVMBySnapshotId(Long id) {
         VMSnapshotVO vmSnapshot = _vmSnapshotDao.findById(id);
+        if(vmSnapshot == null){
+            throw new InvalidParameterValueException("unable to find the vm snapshot with id " + id);
+        }
         Long vmId = vmSnapshot.getVmId();
         UserVmVO vm = _userVMDao.findById(vmId);
         return vm;
@@ -790,37 +804,29 @@ public class VMSnapshotManagerImpl implements VMSnapshotManager, VMSnapshotServi
     }
 
     @Override
-    public boolean syncVMSnapshot(VMInstanceVO vm, VMSnapshotVO vmSnapshot, Long hostId) {
+    public boolean syncVMSnapshot(VMInstanceVO vm, Long hostId) {
         try{
             
             UserVmVO userVm = _userVMDao.findById(vm.getId());
             if(userVm == null)
                 return false;
-            if(userVm.getState() == State.RevertingToRunning || userVm.getState() == State.RevertingToStopped){
-                List<VMSnapshotVO> vmSnapshots = _vmSnapshotDao.listByInstanceId(userVm.getId(), VMSnapshot.State.Reverting);
-                if(vmSnapshots.size() != 1){
-                    s_logger.error("VM:" + userVm.getInstanceName()+ " expected to have one VM snapshot in reverting state but actually has " + vmSnapshots.size());
-                    return false;
-                }
-                // send revert command again
-                revertInternal(userVm, vmSnapshots.get(0), hostId);
-                return true;
-            }else if (userVm.getState() == State.RunningSnapshotting || userVm.getState() == State.StoppedSnapshotting){
-                List<VMSnapshotVO> vmSnapshots = _vmSnapshotDao.listByInstanceId(userVm.getId(), VMSnapshot.State.Creating);
-                if(vmSnapshots.size() != 1){
-                    s_logger.error("VM:" + userVm.getInstanceName()+ " expected to have one VM snapshot in creating state but actually has " + vmSnapshots.size());
-                    return false;
+            
+            List<VMSnapshotVO> vmSnapshotsInExpungingStates = _vmSnapshotDao.listByInstanceId(vm.getId(), VMSnapshot.State.Expunging, VMSnapshot.State.Reverting, VMSnapshot.State.Creating);
+            for (VMSnapshotVO vmSnapshotVO : vmSnapshotsInExpungingStates) {
+                if(vmSnapshotVO.getState() == VMSnapshot.State.Expunging){
+                    return deleteSnapshotInternal(vmSnapshotVO);
+                }else if(vmSnapshotVO.getState() == VMSnapshot.State.Creating){
+                    return createVmSnapshotInternal(userVm, vmSnapshotVO, hostId) != null;
+                }else if(vmSnapshotVO.getState() == VMSnapshot.State.Reverting){
+                    return revertInternal(userVm, vmSnapshotVO, hostId) != null;
                 }
-                // send create vm snapshot command again
-                createVmSnapshotInternal(userVm, vmSnapshots.get(0), hostId);
-                return true;
-                
-            }else if (vmSnapshot != null && vmSnapshot.getState() == VMSnapshot.State.Expunging){
-                return deleteSnapshotInternal(vmSnapshot);
             }
         }catch (Exception e) {
             s_logger.error(e.getMessage(),e);
-            return false;
+            if(_vmSnapshotDao.listByInstanceId(vm.getId(), VMSnapshot.State.Expunging).size() == 0)
+                return true;
+            else
+                return false;
         }
         return false;
     }

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/server/test/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/vm/snapshot/VMSnapshotManagerTest.java b/server/test/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
index fe49207..a97e9b9 100644
--- a/server/test/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
+++ b/server/test/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
@@ -16,17 +16,27 @@
 // under the License.
 package com.cloud.vm.snapshot;
 
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
-import org.apache.cloudstack.api.command.user.vmsnapshot.CreateVMSnapshotCmd;
+import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.Spy;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.Answer;
-import com.cloud.agent.api.Command;
 import com.cloud.agent.api.CreateVMSnapshotAnswer;
 import com.cloud.agent.api.CreateVMSnapshotCommand;
 import com.cloud.agent.api.to.VolumeTO;
@@ -49,25 +59,15 @@ import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
 import com.cloud.user.dao.AccountDao;
 import com.cloud.user.dao.UserDao;
-import com.cloud.uservm.UserVm;
 import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
 import com.cloud.vm.UserVmVO;
-import com.cloud.vm.VMInstanceVO;
-import com.cloud.vm.VirtualMachine;
-import com.cloud.vm.VirtualMachine.Event;
 import com.cloud.vm.VirtualMachine.State;
 import com.cloud.vm.VirtualMachineManager;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.VMInstanceDao;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
 
-import static org.junit.Assert.*;
-import static org.mockito.Mockito.*;
-import org.junit.Before;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
-import org.mockito.Spy;
-
 
 
 public class VMSnapshotManagerTest {
@@ -90,8 +90,6 @@ public class VMSnapshotManagerTest {
     @Mock ConfigurationDao _configDao;
     int _vmSnapshotMax = 10;
     
-    @Mock CreateVMSnapshotCmd createCmd;
-    
     private static long TEST_VM_ID = 3L;
     @Mock UserVmVO vmMock;
     @Mock VolumeVO volumeMock;
@@ -113,9 +111,6 @@ public class VMSnapshotManagerTest {
         
         _vmSnapshotMgr._vmSnapshotMax = _vmSnapshotMax;
         
-        when(createCmd.getVmId()).thenReturn(TEST_VM_ID);
-        when(createCmd.getDisplayName()).thenReturn("testName");
-        
         when(_userVMDao.findById(anyLong())).thenReturn(vmMock);
         when(_vmSnapshotDao.findByName(anyLong(), anyString())).thenReturn(null);
         when(_vmSnapshotDao.findByVm(anyLong())).thenReturn(new ArrayList<VMSnapshotVO>());
@@ -135,48 +130,57 @@ public class VMSnapshotManagerTest {
     @Test(expected=InvalidParameterValueException.class)
     public void testAllocVMSnapshotF1() throws ResourceAllocationException{
         when(_userVMDao.findById(TEST_VM_ID)).thenReturn(null);
-        _vmSnapshotMgr.allocVMSnapshot(createCmd);
+        _vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID,"","",true);
     }
 
     // vm state not in [running, stopped] case
     @Test(expected=InvalidParameterValueException.class)
     public void testAllocVMSnapshotF2() throws ResourceAllocationException{
         when(vmMock.getState()).thenReturn(State.Starting);
-        _vmSnapshotMgr.allocVMSnapshot(createCmd);
+        _vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID,"","",true);
+    }
+    
+    // VM in stopped state & snapshotmemory case
+    @Test(expected=InvalidParameterValueException.class)
+    public void testCreateVMSnapshotF3() throws AgentUnavailableException, OperationTimedoutException, ResourceAllocationException{
+        when(vmMock.getState()).thenReturn(State.Stopped);
+        _vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID,"","",true);
     }
     
     // max snapshot limit case
     @SuppressWarnings("unchecked")
     @Test(expected=CloudRuntimeException.class)
-    public void testAllocVMSnapshotF3() throws ResourceAllocationException{
+    public void testAllocVMSnapshotF4() throws ResourceAllocationException{
         List<VMSnapshotVO> mockList = mock(List.class);
         when(mockList.size()).thenReturn(10);
         when(_vmSnapshotDao.findByVm(TEST_VM_ID)).thenReturn(mockList);
-        _vmSnapshotMgr.allocVMSnapshot(createCmd);
+        _vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID,"","",true);
     }
     
     // active volume snapshots case
     @SuppressWarnings("unchecked")
     @Test(expected=CloudRuntimeException.class)
-    public void testAllocVMSnapshotF4() throws ResourceAllocationException{
+    public void testAllocVMSnapshotF5() throws ResourceAllocationException{
         List<SnapshotVO> mockList = mock(List.class);
         when(mockList.size()).thenReturn(1);
         when(_snapshotDao.listByInstanceId(TEST_VM_ID,Snapshot.Status.Creating,
                 Snapshot.Status.CreatedOnPrimary, Snapshot.Status.BackingUp)).thenReturn(mockList);
-        _vmSnapshotMgr.allocVMSnapshot(createCmd);
+        _vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID,"","",true);
     }
     
+    // successful creation case
     @Test
-    public void testCreateVMSnapshot() throws AgentUnavailableException, OperationTimedoutException{
+    public void testCreateVMSnapshot() throws AgentUnavailableException, OperationTimedoutException, ResourceAllocationException, NoTransitionException{
+        when(vmMock.getState()).thenReturn(State.Running);
+        _vmSnapshotMgr.allocVMSnapshot(TEST_VM_ID,"","",true);
+        
         when(_vmSnapshotDao.findCurrentSnapshotByVmId(anyLong())).thenReturn(null);
         doReturn(new ArrayList<VolumeTO>()).when(_vmSnapshotMgr).getVolumeTOList(anyLong());
         doReturn(new CreateVMSnapshotAnswer(null,true,"")).when(_vmSnapshotMgr).sendToPool(anyLong(), any(CreateVMSnapshotCommand.class));
-        doReturn(true).when(_vmSnapshotMgr).transitState(any(VMSnapshotVO.class), 
-                any(VMSnapshot.Event.class), any(VMInstanceVO.class), any(VirtualMachine.Event.class), anyLong());
         doNothing().when(_vmSnapshotMgr).processAnswer(any(VMSnapshotVO.class), 
                 any(UserVmVO.class), any(Answer.class), anyLong());
+        doReturn(true).when(_vmSnapshotMgr).vmSnapshotStateTransitTo(any(VMSnapshotVO.class),any(VMSnapshot.Event.class));
         _vmSnapshotMgr.createVmSnapshotInternal(vmMock, mock(VMSnapshotVO.class), 5L);
-        
     }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/ebca6890/ui/scripts/instances.js
----------------------------------------------------------------------
diff --git a/ui/scripts/instances.js b/ui/scripts/instances.js
index d9542a1..264b5a1 100644
--- a/ui/scripts/instances.js
+++ b/ui/scripts/instances.js
@@ -49,14 +49,6 @@
         zonename: { label: 'label.zone.name' },
         state: {
           label: 'label.state',
-          converter: function(data) {
-            if(data == 'StoppedSnapshotting' || data == 'RunningSnapshotting')
-              return  'Snapshotting';
-            if(data == 'RevertingToRunning' || data == 'RevertingToStopped'){
-              return 'Reverting';
-            } 
-            return data;
-          },          
           indicator: {
             'Running': 'on',
             'Stopped': 'off',


Mime
View raw message