You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ed...@apache.org on 2013/10/26 03:24:55 UTC

[1/5] move a lot of code into vmsnapshot strategy

Updated Branches:
  refs/heads/master 718bc37dc -> 465c9ec5c


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/snapshot/test/src/VMSnapshotStrategyTest.java
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/test/src/VMSnapshotStrategyTest.java b/engine/storage/snapshot/test/src/VMSnapshotStrategyTest.java
new file mode 100644
index 0000000..8e36faf
--- /dev/null
+++ b/engine/storage/snapshot/test/src/VMSnapshotStrategyTest.java
@@ -0,0 +1,256 @@
+/*
+ * 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 src;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.cloudstack.storage.vmsnapshot.DefaultVMSnapshotStrategy;
+import org.apache.cloudstack.storage.vmsnapshot.VMSnapshotHelper;
+import org.apache.cloudstack.test.utils.SpringUtils;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.ComponentScan;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.FilterType;
+import org.springframework.core.type.classreading.MetadataReader;
+import org.springframework.core.type.classreading.MetadataReaderFactory;
+import org.springframework.core.type.filter.TypeFilter;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.support.AnnotationConfigContextLoader;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Command;
+import com.cloud.agent.api.CreateVMSnapshotAnswer;
+import com.cloud.agent.api.DeleteVMSnapshotAnswer;
+import com.cloud.agent.api.RevertToVMSnapshotAnswer;
+import com.cloud.agent.api.VMSnapshotTO;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.storage.GuestOSVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.dao.GuestOSDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.component.ComponentContext;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+
+import junit.framework.TestCase;
+
+@RunWith(SpringJUnit4ClassRunner.class)
+@ContextConfiguration(loader = AnnotationConfigContextLoader.class)
+public class VMSnapshotStrategyTest extends TestCase {
+    @Inject
+    VMSnapshotStrategy vmSnapshotStrategy;
+    @Inject
+    VMSnapshotHelper vmSnapshotHelper;
+    @Inject UserVmDao userVmDao;
+    @Inject
+    GuestOSDao guestOSDao;
+    @Inject
+    AgentManager agentMgr;
+    @Inject
+    VMSnapshotDao vmSnapshotDao;
+    @Override
+    @Before
+    public void setUp() {
+        ComponentContext.initComponentsLifeCycle();
+    }
+
+
+    @Test
+    public void testCreateVMSnapshot() throws AgentUnavailableException, OperationTimedoutException {
+        Long hostId = 1L;
+        Long vmId = 1L;
+        Long guestOsId = 1L;
+        List<VolumeObjectTO> volumeObjectTOs = new ArrayList<VolumeObjectTO>();
+        VMSnapshotVO vmSnapshot = Mockito.mock(VMSnapshotVO.class);
+        UserVmVO userVmVO = Mockito.mock(UserVmVO.class);
+        Mockito.when(userVmVO.getGuestOSId()).thenReturn(guestOsId);
+        Mockito.when(vmSnapshot.getVmId()).thenReturn(vmId);
+        Mockito.when(vmSnapshotHelper.pickRunningHost(Mockito.anyLong())).thenReturn(hostId);
+        Mockito.when(vmSnapshotHelper.getVolumeTOList(Mockito.anyLong())).thenReturn(volumeObjectTOs);
+        Mockito.when(userVmDao.findById(Mockito.anyLong())).thenReturn(userVmVO);
+        GuestOSVO guestOSVO = Mockito.mock(GuestOSVO.class);
+        Mockito.when(guestOSDao.findById(Mockito.anyLong())).thenReturn(guestOSVO);
+        Mockito.when(agentMgr.send(Mockito.anyLong(), Mockito.any(Command.class))).thenReturn(null);
+        Exception e = null;
+        try {
+            vmSnapshotStrategy.takeVMSnapshot(vmSnapshot);
+        } catch (CloudRuntimeException e1) {
+            e = e1;
+        }
+
+        assertNotNull(e);
+        CreateVMSnapshotAnswer answer = Mockito.mock(CreateVMSnapshotAnswer.class);
+        Mockito.when(answer.getResult()).thenReturn(true);
+        Mockito.when(agentMgr.send(Mockito.anyLong(), Mockito.any(Command.class))).thenReturn(answer);
+        Mockito.when(vmSnapshotDao.findById(Mockito.anyLong())).thenReturn(vmSnapshot);
+        VMSnapshot snapshot = null;
+        snapshot = vmSnapshotStrategy.takeVMSnapshot(vmSnapshot);
+        assertNotNull(snapshot);
+    }
+
+    @Test
+    public void testRevertSnapshot() throws AgentUnavailableException, OperationTimedoutException {
+        Long hostId = 1L;
+        Long vmId = 1L;
+        Long guestOsId = 1L;
+        List<VolumeObjectTO> volumeObjectTOs = new ArrayList<VolumeObjectTO>();
+        VMSnapshotVO vmSnapshot = Mockito.mock(VMSnapshotVO.class);
+        UserVmVO userVmVO = Mockito.mock(UserVmVO.class);
+        Mockito.when(userVmVO.getGuestOSId()).thenReturn(guestOsId);
+        Mockito.when(vmSnapshot.getVmId()).thenReturn(vmId);
+        Mockito.when(vmSnapshotHelper.pickRunningHost(Mockito.anyLong())).thenReturn(hostId);
+        Mockito.when(vmSnapshotHelper.getVolumeTOList(Mockito.anyLong())).thenReturn(volumeObjectTOs);
+        Mockito.when(userVmDao.findById(Mockito.anyLong())).thenReturn(userVmVO);
+        GuestOSVO guestOSVO = Mockito.mock(GuestOSVO.class);
+        Mockito.when(guestOSDao.findById(Mockito.anyLong())).thenReturn(guestOSVO);
+        VMSnapshotTO vmSnapshotTO = Mockito.mock(VMSnapshotTO.class);
+        Mockito.when(vmSnapshotHelper.getSnapshotWithParents(Mockito.any(VMSnapshotVO.class))).thenReturn(vmSnapshotTO);
+        Mockito.when(vmSnapshotDao.findById(Mockito.anyLong())).thenReturn(vmSnapshot);
+        Mockito.when(vmSnapshot.getId()).thenReturn(1L);
+        Mockito.when(vmSnapshot.getCreated()).thenReturn(new Date());
+        Mockito.when(agentMgr.send(Mockito.anyLong(), Mockito.any(Command.class))).thenReturn(null);
+        Exception e = null;
+        try {
+            vmSnapshotStrategy.revertVMSnapshot(vmSnapshot);
+        } catch (CloudRuntimeException e1) {
+            e = e1;
+        }
+
+        assertNotNull(e);
+
+        RevertToVMSnapshotAnswer answer = Mockito.mock(RevertToVMSnapshotAnswer.class);
+        Mockito.when(answer.getResult()).thenReturn(Boolean.TRUE);
+        Mockito.when(agentMgr.send(Mockito.anyLong(), Mockito.any(Command.class))).thenReturn(answer);
+        boolean result = vmSnapshotStrategy.revertVMSnapshot(vmSnapshot);
+        assertTrue(result);
+    }
+
+    @Test
+    public void testDeleteVMSnapshot() throws AgentUnavailableException, OperationTimedoutException {
+        Long hostId = 1L;
+        Long vmId = 1L;
+        Long guestOsId = 1L;
+        List<VolumeObjectTO> volumeObjectTOs = new ArrayList<VolumeObjectTO>();
+        VMSnapshotVO vmSnapshot = Mockito.mock(VMSnapshotVO.class);
+        UserVmVO userVmVO = Mockito.mock(UserVmVO.class);
+        Mockito.when(userVmVO.getGuestOSId()).thenReturn(guestOsId);
+        Mockito.when(vmSnapshot.getVmId()).thenReturn(vmId);
+        Mockito.when(vmSnapshotHelper.pickRunningHost(Mockito.anyLong())).thenReturn(hostId);
+        Mockito.when(vmSnapshotHelper.getVolumeTOList(Mockito.anyLong())).thenReturn(volumeObjectTOs);
+        Mockito.when(userVmDao.findById(Mockito.anyLong())).thenReturn(userVmVO);
+        GuestOSVO guestOSVO = Mockito.mock(GuestOSVO.class);
+        Mockito.when(guestOSDao.findById(Mockito.anyLong())).thenReturn(guestOSVO);
+        VMSnapshotTO vmSnapshotTO = Mockito.mock(VMSnapshotTO.class);
+        Mockito.when(vmSnapshotHelper.getSnapshotWithParents(Mockito.any(VMSnapshotVO.class))).thenReturn(vmSnapshotTO);
+        Mockito.when(vmSnapshotDao.findById(Mockito.anyLong())).thenReturn(vmSnapshot);
+        Mockito.when(vmSnapshot.getId()).thenReturn(1L);
+        Mockito.when(vmSnapshot.getCreated()).thenReturn(new Date());
+        Mockito.when(agentMgr.send(Mockito.anyLong(), Mockito.any(Command.class))).thenReturn(null);
+
+        Exception e = null;
+        try {
+            vmSnapshotStrategy.deleteVMSnapshot(vmSnapshot);
+        } catch (CloudRuntimeException e1) {
+            e = e1;
+        }
+
+        assertNotNull(e);
+
+        DeleteVMSnapshotAnswer answer = Mockito.mock(DeleteVMSnapshotAnswer.class);
+        Mockito.when(answer.getResult()).thenReturn(true);
+        Mockito.when(agentMgr.send(Mockito.anyLong(), Mockito.any(Command.class))).thenReturn(answer);
+
+        boolean result = vmSnapshotStrategy.deleteVMSnapshot(vmSnapshot);
+        assertTrue(result);
+    }
+
+
+    @Configuration
+    @ComponentScan(basePackageClasses = {NetUtils.class, DefaultVMSnapshotStrategy.class}, includeFilters = {@ComponentScan.Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)}, useDefaultFilters = false)
+    public static class TestConfiguration extends SpringUtils.CloudStackTestConfiguration {
+
+        public static class Library implements TypeFilter {
+            @Override
+            public boolean match(MetadataReader mdr, MetadataReaderFactory arg1) throws IOException {
+                mdr.getClassMetadata().getClassName();
+                ComponentScan cs = TestConfiguration.class.getAnnotation(ComponentScan.class);
+                return SpringUtils.includedInBasePackageClasses(mdr.getClassMetadata().getClassName(), cs);
+            }
+        }
+
+        @Bean
+        public VMSnapshotHelper vmSnapshotHelper() {
+            return Mockito.mock(VMSnapshotHelper.class);
+        }
+
+        @Bean
+        public GuestOSDao guestOSDao() {
+            return Mockito.mock(GuestOSDao.class);
+        }
+
+        @Bean
+        public UserVmDao userVmDao() {
+            return Mockito.mock(UserVmDao.class);
+        }
+
+        @Bean
+        public VMSnapshotDao vmSnapshotDao() {
+            return Mockito.mock(VMSnapshotDao.class);
+        }
+
+        @Bean
+        public ConfigurationDao configurationDao() {
+            return Mockito.mock(ConfigurationDao.class);
+        }
+
+        @Bean
+        public AgentManager agentManager() {
+            return Mockito.mock(AgentManager.class);
+        }
+
+        @Bean
+        public VolumeDao volumeDao() {
+            return Mockito.mock(VolumeDao.class);
+        }
+
+        @Bean
+        public DiskOfferingDao diskOfferingDao() {
+            return Mockito.mock(DiskOfferingDao.class);
+        }
+    }
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
index c902aef..376baa5 100644
--- a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
+++ b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
@@ -18,16 +18,37 @@
  */
 package org.apache.cloudstack.storage.volume;
 
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.List;
-import java.util.Map;
-
-import javax.inject.Inject;
-
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
-
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.storage.ListVolumeAnswer;
+import com.cloud.agent.api.storage.ListVolumeCommand;
+import com.cloud.agent.api.to.VirtualMachineTO;
+import com.cloud.alert.AlertManager;
+import com.cloud.configuration.Config;
+import com.cloud.configuration.Resource.ResourceType;
+import com.cloud.event.EventTypes;
+import com.cloud.event.UsageEventUtils;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.host.Host;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.VMTemplateStoragePoolVO;
+import com.cloud.storage.VMTemplateStorageResourceAssoc;
+import com.cloud.storage.VMTemplateStorageResourceAssoc.Status;
+import com.cloud.storage.Volume;
+import com.cloud.storage.Volume.State;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.VMTemplatePoolDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.storage.snapshot.SnapshotManager;
+import com.cloud.storage.template.TemplateProp;
+import com.cloud.user.AccountManager;
+import com.cloud.user.ResourceLimitService;
+import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.GlobalLock;
+import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.cloudstack.engine.cloud.entity.api.VolumeEntity;
 import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
@@ -55,45 +76,19 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.storage.command.CommandResult;
 import org.apache.cloudstack.storage.command.CopyCmdAnswer;
 import org.apache.cloudstack.storage.command.DeleteCommand;
-import org.apache.cloudstack.storage.datastore.DataObjectManager;
-import org.apache.cloudstack.storage.datastore.ObjectInDataStoreManager;
 import org.apache.cloudstack.storage.datastore.PrimaryDataStore;
 import org.apache.cloudstack.storage.datastore.PrimaryDataStoreProviderManager;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
 
-import com.cloud.agent.api.Answer;
-import com.cloud.agent.api.storage.ListVolumeAnswer;
-import com.cloud.agent.api.storage.ListVolumeCommand;
-import com.cloud.agent.api.to.VirtualMachineTO;
-import com.cloud.alert.AlertManager;
-import com.cloud.configuration.Config;
-import com.cloud.configuration.Resource.ResourceType;
-import com.cloud.event.EventTypes;
-import com.cloud.event.UsageEventUtils;
-import com.cloud.exception.ConcurrentOperationException;
-import com.cloud.exception.ResourceAllocationException;
-import com.cloud.host.Host;
-import com.cloud.storage.DataStoreRole;
-import com.cloud.storage.ScopeType;
-import com.cloud.storage.StoragePool;
-import com.cloud.storage.VMTemplateStoragePoolVO;
-import com.cloud.storage.VMTemplateStorageResourceAssoc;
-import com.cloud.storage.VMTemplateStorageResourceAssoc.Status;
-import com.cloud.storage.Volume;
-import com.cloud.storage.Volume.State;
-import com.cloud.storage.VolumeVO;
-import com.cloud.storage.dao.VMTemplatePoolDao;
-import com.cloud.storage.dao.VolumeDao;
-import com.cloud.storage.snapshot.SnapshotManager;
-import com.cloud.storage.template.TemplateProp;
-import com.cloud.user.AccountManager;
-import com.cloud.user.ResourceLimitService;
-import com.cloud.utils.NumbersUtil;
-import com.cloud.utils.db.DB;
-import com.cloud.utils.db.GlobalLock;
-import com.cloud.utils.exception.CloudRuntimeException;
+import javax.inject.Inject;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
 
 @Component
 public class VolumeServiceImpl implements VolumeService {
@@ -103,10 +98,6 @@ public class VolumeServiceImpl implements VolumeService {
     @Inject
     PrimaryDataStoreProviderManager dataStoreMgr;
     @Inject
-    ObjectInDataStoreManager objectInDataStoreMgr;
-    @Inject
-    DataObjectManager dataObjectMgr;
-    @Inject
     DataMotionService motionSrv;
     @Inject
     VolumeDataFactory volFactory;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java
index e11e766..0e2423e 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareStorageManagerImpl.java
@@ -16,39 +16,6 @@
 // under the License.
 package com.cloud.hypervisor.vmware.manager;
 
-import java.io.BufferedWriter;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
-import java.io.OutputStreamWriter;
-import java.rmi.RemoteException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Properties;
-import java.util.UUID;
-
-import org.apache.log4j.Logger;
-
-import com.vmware.vim25.FileInfo;
-import com.vmware.vim25.FileQueryFlags;
-import com.vmware.vim25.HostDatastoreBrowserSearchResults;
-import com.vmware.vim25.HostDatastoreBrowserSearchSpec;
-import com.vmware.vim25.ManagedObjectReference;
-import com.vmware.vim25.TaskInfo;
-import com.vmware.vim25.VirtualDeviceConfigSpec;
-import com.vmware.vim25.VirtualDeviceConfigSpecOperation;
-import com.vmware.vim25.VirtualDisk;
-import com.vmware.vim25.VirtualLsiLogicController;
-import com.vmware.vim25.VirtualMachineConfigSpec;
-import com.vmware.vim25.VirtualMachineFileInfo;
-import com.vmware.vim25.VirtualMachineGuestOsIdentifier;
-import com.vmware.vim25.VirtualSCSISharing;
-
-import org.apache.cloudstack.storage.to.TemplateObjectTO;
-import org.apache.cloudstack.storage.to.VolumeObjectTO;
-
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.BackupSnapshotAnswer;
 import com.cloud.agent.api.BackupSnapshotCommand;
@@ -73,7 +40,6 @@ import com.cloud.agent.api.to.DataStoreTO;
 import com.cloud.agent.api.to.DataTO;
 import com.cloud.agent.api.to.NfsTO;
 import com.cloud.agent.api.to.StorageFilerTO;
-import com.cloud.agent.api.to.VolumeTO;
 import com.cloud.hypervisor.vmware.mo.CustomFieldConstants;
 import com.cloud.hypervisor.vmware.mo.DatacenterMO;
 import com.cloud.hypervisor.vmware.mo.DatastoreMO;
@@ -97,6 +63,29 @@ import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.snapshot.VMSnapshot;
+import com.vmware.vim25.FileInfo;
+import com.vmware.vim25.FileQueryFlags;
+import com.vmware.vim25.HostDatastoreBrowserSearchResults;
+import com.vmware.vim25.HostDatastoreBrowserSearchSpec;
+import com.vmware.vim25.ManagedObjectReference;
+import com.vmware.vim25.TaskInfo;
+import com.vmware.vim25.VirtualDisk;
+import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.log4j.Logger;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.OutputStreamWriter;
+import java.rmi.RemoteException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.UUID;
 
 public class VmwareStorageManagerImpl implements VmwareStorageManager {
     @Override
@@ -1280,7 +1269,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
 
     @Override
     public CreateVMSnapshotAnswer execute(VmwareHostService hostService, CreateVMSnapshotCommand cmd) {
-        List<VolumeTO> volumeTOs = cmd.getVolumeTOs();
+        List<VolumeObjectTO> volumeTOs = cmd.getVolumeTOs();
         String vmName = cmd.getVmName();
         String vmSnapshotName = cmd.getTarget().getSnapshotName();
         String vmSnapshotDesc = cmd.getTarget().getDescription();
@@ -1330,19 +1319,20 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
                         mapNewDisk.put(baseName, vmdkName);
                     }
                 }
-                for (VolumeTO volumeTO : volumeTOs) {
+                for (VolumeObjectTO volumeTO : volumeTOs) {
                     String baseName = extractSnapshotBaseFileName(volumeTO.getPath());
                     String newPath = mapNewDisk.get(baseName);
                     // get volume's chain size for this VM snapshot, exclude current volume vdisk
+                    DataStoreTO store = volumeTO.getDataStore();
                     long size = getVMSnapshotChainSize(context,hyperHost,baseName + "*.vmdk",
-                            volumeTO.getPoolUuid(), newPath);
+                            store.getUuid(), newPath);
 
-                    if(volumeTO.getType()== Volume.Type.ROOT){
+                    if(volumeTO.getVolumeType()== Volume.Type.ROOT){
                         // add memory snapshot size
-                        size = size + getVMSnapshotChainSize(context,hyperHost,cmd.getVmName()+"*.vmsn",volumeTO.getPoolUuid(),null);
+                        size = size + getVMSnapshotChainSize(context,hyperHost,cmd.getVmName()+"*.vmsn",store.getUuid(),null);
                     }
 
-                    volumeTO.setChainSize(size);
+                    volumeTO.setSize(size);
                     volumeTO.setPath(newPath);
                 }
                 return new CreateVMSnapshotAnswer(cmd, cmd.getTarget(), volumeTOs);
@@ -1362,7 +1352,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
 
     @Override
     public DeleteVMSnapshotAnswer execute(VmwareHostService hostService, DeleteVMSnapshotCommand cmd) {
-        List<VolumeTO> listVolumeTo = cmd.getVolumeTOs();
+        List<VolumeObjectTO> listVolumeTo = cmd.getVolumeTOs();
         VirtualMachineMO vmMo = null;
         VmwareContext context = hostService.getServiceContext(cmd);
         Map<String, String> mapNewDisk = new HashMap<String, String>();
@@ -1403,16 +1393,17 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
                         mapNewDisk.put(baseName, vmdkName);
                     }
                 }
-                for (VolumeTO volumeTo : listVolumeTo) {
+                for (VolumeObjectTO volumeTo : listVolumeTo) {
                     String baseName = extractSnapshotBaseFileName(volumeTo.getPath());
                     String newPath = mapNewDisk.get(baseName);
+                    DataStoreTO store = volumeTo.getDataStore();
                     long size = getVMSnapshotChainSize(context,hyperHost,
-                            baseName + "*.vmdk", volumeTo.getPoolUuid(), newPath);
-                    if(volumeTo.getType()== Volume.Type.ROOT){
+                            baseName + "*.vmdk", store.getUuid(), newPath);
+                    if(volumeTo.getVolumeType()== Volume.Type.ROOT){
                         // add memory snapshot size
-                        size = size + getVMSnapshotChainSize(context,hyperHost,cmd.getVmName()+"*.vmsn",volumeTo.getPoolUuid(),null);
+                        size = size + getVMSnapshotChainSize(context,hyperHost,cmd.getVmName()+"*.vmsn",volumeTo.getUuid(),null);
                     }
-                    volumeTo.setChainSize(size);
+                    volumeTo.setSize(size);
                     volumeTo.setPath(newPath);
                 }
                 return new DeleteVMSnapshotAnswer(cmd, listVolumeTo);
@@ -1429,7 +1420,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
         String snapshotName = cmd.getTarget().getSnapshotName();
         String vmName = cmd.getVmName();
         Boolean snapshotMemory = cmd.getTarget().getType() == VMSnapshot.Type.DiskAndMemory;
-        List<VolumeTO> listVolumeTo = cmd.getVolumeTOs();
+        List<VolumeObjectTO> listVolumeTo = cmd.getVolumeTOs();
         VirtualMachine.State vmState = VirtualMachine.State.Running;
         VirtualMachineMO vmMo = null;
         VmwareContext context = hostService.getServiceContext(cmd);
@@ -1483,7 +1474,7 @@ public class VmwareStorageManagerImpl implements VmwareStorageManager {
                         }
                     }
                     String key = null;
-                    for (VolumeTO volumeTo : listVolumeTo) {
+                    for (VolumeObjectTO volumeTo : listVolumeTo) {
                         String parentUUID = volumeTo.getPath();
                         String[] s = parentUUID.split("-");
                         key = s[0];

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
index 0ac8b1c..c6a35cc 100644
--- a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
+++ b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
@@ -734,7 +734,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
 
     private Answer execute(RevertToVMSnapshotCommand cmd) {
         String vmName = cmd.getVmName();
-        List<VolumeTO> listVolumeTo = cmd.getVolumeTOs();
+        List<VolumeObjectTO> listVolumeTo = cmd.getVolumeTOs();
         VMSnapshot.Type vmSnapshotType = cmd.getTarget().getType();
         Boolean snapshotMemory = vmSnapshotType == VMSnapshot.Type.DiskAndMemory;
         Connection conn = getConnection();
@@ -786,7 +786,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
             }
 
             // after revert, VM's volumes path have been changed, need to report to manager
-            for (VolumeTO volumeTo : listVolumeTo) {
+            for (VolumeObjectTO volumeTo : listVolumeTo) {
                 Long deviceId = volumeTo.getDeviceId();
                 VDI vdi = vdiMap.get(deviceId.toString());
                 volumeTo.setPath(vdi.getUuid(conn));
@@ -6633,7 +6633,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
 
     }
 
-    private long getVMSnapshotChainSize(Connection conn, VolumeTO volumeTo, String vmName) 
+    private long getVMSnapshotChainSize(Connection conn, VolumeObjectTO volumeTo, String vmName)
             throws BadServerResponse, XenAPIException, XmlRpcException {
         Set<VDI> allvolumeVDIs = VDI.getByNameLabel(conn, volumeTo.getName());
         long size = 0;
@@ -6657,7 +6657,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
                 continue;
             }
         }
-        if (volumeTo.getType() == Volume.Type.ROOT) {
+        if (volumeTo.getVolumeType() == Volume.Type.ROOT) {
             Map<VM, VM.Record> allVMs = VM.getAllRecords(conn);
             // add size of memory snapshot vdi
             if (allVMs.size() > 0) {
@@ -6690,7 +6690,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
     protected Answer execute(final CreateVMSnapshotCommand cmd) {
         String vmName = cmd.getVmName();
         String vmSnapshotName = cmd.getTarget().getSnapshotName();
-        List<VolumeTO> listVolumeTo = cmd.getVolumeTOs();
+        List<VolumeObjectTO> listVolumeTo = cmd.getVolumeTOs();
         VirtualMachine.State vmState = cmd.getVmState();
         String guestOSType = cmd.getGuestOSType();
 
@@ -6770,9 +6770,9 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
 
             }
             // calculate used capacity for this VM snapshot
-            for (VolumeTO volumeTo : cmd.getVolumeTOs()){
+            for (VolumeObjectTO volumeTo : cmd.getVolumeTOs()){
                 long size = getVMSnapshotChainSize(conn,volumeTo,cmd.getVmName());
-                volumeTo.setChainSize(size);
+                volumeTo.setSize(size);
             }
             
             success = true;
@@ -6821,7 +6821,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
     }
 
     private VM createWorkingVM(Connection conn, String vmName,
-            String guestOSType, List<VolumeTO> listVolumeTo)
+            String guestOSType, List<VolumeObjectTO> listVolumeTo)
                     throws BadServerResponse, VmBadPowerState, SrFull,
                     OperationNotAllowed, XenAPIException, XmlRpcException {
         String guestOsTypeName = getGuestOsType(guestOSType, false);
@@ -6835,8 +6835,8 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
         VM template = getVM(conn, guestOsTypeName);
         VM vm = template.createClone(conn, vmName);
         vm.setIsATemplate(conn, false);
-        Map<VDI, VolumeTO> vdiMap = new HashMap<VDI, VolumeTO>();
-        for (VolumeTO volume : listVolumeTo) {
+        Map<VDI, VolumeObjectTO> vdiMap = new HashMap<VDI, VolumeObjectTO>();
+        for (VolumeObjectTO volume : listVolumeTo) {
             String vdiUuid = volume.getPath();
             try {
                 VDI vdi = VDI.getByUuid(conn, vdiUuid);
@@ -6847,11 +6847,11 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
             }
         }
         for (VDI vdi : vdiMap.keySet()) {
-            VolumeTO volumeTO = vdiMap.get(vdi);
+            VolumeObjectTO volumeTO = vdiMap.get(vdi);
             VBD.Record vbdr = new VBD.Record();
             vbdr.VM = vm;
             vbdr.VDI = vdi;
-            if (volumeTO.getType() == Volume.Type.ROOT) {
+            if (volumeTO.getVolumeType() == Volume.Type.ROOT) {
                 vbdr.bootable = true;
                 vbdr.unpluggable = false;
             } else {
@@ -6898,9 +6898,9 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
 
             }
             // re-calculate used capacify for this VM snapshot
-            for (VolumeTO volumeTo : cmd.getVolumeTOs()){
+            for (VolumeObjectTO volumeTo : cmd.getVolumeTOs()){
                 long size = getVMSnapshotChainSize(conn,volumeTo,cmd.getVmName());
-                volumeTo.setChainSize(size);
+                volumeTo.setSize(size);
             }
             
             return new DeleteVMSnapshotAnswer(cmd, cmd.getVolumeTOs());

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
index dade983..9de0031 100755
--- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -342,8 +342,6 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
         return snapshot;
     }
 
-
-
     @Override
     public Snapshot backupSnapshot(Long snapshotId) {
         SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId, DataStoreRole.Image);
@@ -354,97 +352,6 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
         return snapshotSrv.backupSnapshot(snapshot);
     }
 
-    /*
-    @Override
-    public void downloadSnapshotsFromSwift(SnapshotVO ss) {
-
-        long volumeId = ss.getVolumeId();
-        VolumeVO volume = _volsDao.findById(volumeId);
-        Long dcId = volume.getDataCenterId();
-        Long accountId = volume.getAccountId();
-        DataStore secStore = this.dataStoreMgr.getImageStore(dcId);
-
-        Long swiftId = ss.getSwiftId();
-        SwiftTO swift = _swiftMgr.getSwiftTO(swiftId);
-        SnapshotVO tss = ss;
-        List<String> BackupUuids = new ArrayList<String>(30);
-        while (true) {
-            BackupUuids.add(0, tss.getBackupSnapshotId());
-            if (tss.getPrevSnapshotId() == 0)
-                break;
-            Long id = tss.getPrevSnapshotId();
-            tss = _snapshotDao.findById(id);
-            assert tss != null : " can not find snapshot " + id;
-        }
-        String parent = null;
-        try {
-            for (String backupUuid : BackupUuids) {
-<<<<<<< HEAD
-                downloadSnapshotFromSwiftCommand cmd = new downloadSnapshotFromSwiftCommand(swift, secStore.getUri(), dcId, accountId, volumeId, parent, backupUuid, _backupsnapshotwait);
-=======
-                 DownloadSnapshotFromSwiftCommand cmd = new DownloadSnapshotFromSwiftCommand(swift, secondaryStoragePoolUrl, dcId, accountId, volumeId, parent, backupUuid, _backupsnapshotwait);
->>>>>>> master
-                Answer answer = _agentMgr.sendToSSVM(dcId, cmd);
-                if ((answer == null) || !answer.getResult()) {
-                    throw new CloudRuntimeException("downloadSnapshotsFromSwift failed ");
-                }
-                parent = backupUuid;
-            }
-        } catch (Exception e) {
-            throw new CloudRuntimeException("downloadSnapshotsFromSwift failed due to " + e.toString());
-        }
-
-    }
-
-    private List<String> determineBackupUuids(final SnapshotVO snapshot) {
-
-        final List<String> backupUuids = new ArrayList<String>();
-        backupUuids.add(0, snapshot.getBackupSnapshotId());
-
-        SnapshotVO tempSnapshot = snapshot;
-        while (tempSnapshot.getPrevSnapshotId() != 0) {
-            tempSnapshot = _snapshotDao.findById(tempSnapshot
-                    .getPrevSnapshotId());
-            backupUuids.add(0, tempSnapshot.getBackupSnapshotId());
-        }
-
-        return Collections.unmodifiableList(backupUuids);
-    }
-
-    @Override
-    public void downloadSnapshotsFromS3(final SnapshotVO snapshot) {
-
-        final VolumeVO volume = _volsDao.findById(snapshot.getVolumeId());
-        final Long zoneId = volume.getDataCenterId();
-        final DataStore secStore = this.dataStoreMgr.getImageStore(zoneId);
-
-        final S3TO s3 = _s3Mgr.getS3TO(snapshot.getS3Id());
-        final List<String> backupUuids = determineBackupUuids(snapshot);
-
-        try {
-            String parent = null;
-            for (final String backupUuid : backupUuids) {
-                final DownloadSnapshotFromS3Command cmd = new DownloadSnapshotFromS3Command(
-                        s3, parent, secStore.getUri(), zoneId,
-                        volume.getAccountId(), volume.getId(), backupUuid,
-                        _backupsnapshotwait);
-                final Answer answer = _agentMgr.sendToSSVM(zoneId, cmd);
-                if ((answer == null) || !answer.getResult()) {
-                    throw new CloudRuntimeException(String.format(
-                            "S3 snapshot download failed due to %1$s.",
-                            answer != null ? answer.getDetails()
-                                    : "unspecified error"));
-                }
-                parent = backupUuid;
-            }
-        } catch (Exception e) {
-            throw new CloudRuntimeException(
-                    "Snapshot download from S3 failed due to " + e.toString(),
-                    e);
-        }
-
-    }*/
-
     @Override
     public SnapshotVO getParentSnapshot(VolumeInfo volume) {
         long preId = _snapshotDao.getLastSnapshot(volume.getId(), DataStoreRole.Primary);
@@ -546,7 +453,6 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
         }
     }
 
-
     @Override
     public String getSecondaryStorageURL(SnapshotVO snapshot) {
         SnapshotDataStoreVO snapshotStore = _snapshotStoreDao.findBySnapshot(snapshot.getId(), DataStoreRole.Image);
@@ -672,7 +578,6 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
         return new Pair<List<? extends Snapshot>, Integer>(result.first(), result.second());
     }
 
-
     @Override
     public boolean deleteSnapshotDirsForAccount(long accountId) {
 
@@ -942,8 +847,6 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
         return null;
     }
 
-
-
     private boolean hostSupportSnapsthotForVolume(HostVO host, VolumeInfo volume) {
         if (host.getHypervisorType() != HypervisorType.KVM) {
             return true;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/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 aa772fe..7a200ff 100644
--- a/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
+++ b/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@ -17,82 +17,34 @@
 
 package com.cloud.vm.snapshot;
 
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import javax.ejb.Local;
-import javax.inject.Inject;
-import javax.naming.ConfigurationException;
-
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
-
-import org.apache.cloudstack.api.command.user.vmsnapshot.ListVMSnapshotCmd;
-import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-
-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.DeleteVMSnapshotAnswer;
-import com.cloud.agent.api.DeleteVMSnapshotCommand;
-import com.cloud.agent.api.RevertToVMSnapshotAnswer;
-import com.cloud.agent.api.RevertToVMSnapshotCommand;
-import com.cloud.agent.api.VMSnapshotTO;
-import com.cloud.agent.api.to.VolumeTO;
 import com.cloud.event.ActionEvent;
 import com.cloud.event.EventTypes;
-import com.cloud.event.UsageEventUtils;
-import com.cloud.exception.AgentUnavailableException;
 import com.cloud.exception.ConcurrentOperationException;
 import com.cloud.exception.InsufficientCapacityException;
 import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.exception.OperationTimedoutException;
 import com.cloud.exception.ResourceAllocationException;
 import com.cloud.exception.ResourceUnavailableException;
-import com.cloud.host.Host;
-import com.cloud.host.HostVO;
-import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import com.cloud.hypervisor.HypervisorGuruManager;
 import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
 import com.cloud.projects.Project.ListProjectResourcesCriteria;
-import com.cloud.service.dao.ServiceOfferingDao;
-import com.cloud.storage.DiskOfferingVO;
-import com.cloud.storage.GuestOSVO;
 import com.cloud.storage.Snapshot;
 import com.cloud.storage.SnapshotVO;
-import com.cloud.storage.StoragePool;
 import com.cloud.storage.VolumeVO;
-import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.storage.dao.GuestOSDao;
 import com.cloud.storage.dao.SnapshotDao;
 import com.cloud.storage.dao.VolumeDao;
 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.DateUtil;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Ternary;
 import com.cloud.utils.component.ManagerBase;
-import com.cloud.utils.db.DB;
 import com.cloud.utils.db.Filter;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
-import com.cloud.utils.db.Transaction;
 import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.fsm.NoTransitionException;
-import com.cloud.utils.fsm.StateMachine2;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
@@ -100,8 +52,22 @@ import com.cloud.vm.VirtualMachine.State;
 import com.cloud.vm.VirtualMachineManager;
 import com.cloud.vm.VirtualMachineProfile;
 import com.cloud.vm.dao.UserVmDao;
-import com.cloud.vm.dao.VMInstanceDao;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import org.apache.cloudstack.api.command.user.vmsnapshot.ListVMSnapshotCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
+import javax.ejb.Local;
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
 @Component
 @Local(value = { VMSnapshotManager.class, VMSnapshotService.class })
@@ -111,25 +77,27 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
     @Inject VMSnapshotDao _vmSnapshotDao;
     @Inject VolumeDao _volumeDao;
     @Inject AccountDao _accountDao;
-    @Inject VMInstanceDao _vmInstanceDao;
     @Inject UserVmDao _userVMDao;
-    @Inject HostDao _hostDao;
-    @Inject UserDao _userDao;
-    @Inject AgentManager _agentMgr;
-    @Inject HypervisorGuruManager _hvGuruMgr;
     @Inject AccountManager _accountMgr;
     @Inject GuestOSDao _guestOSDao;
-    @Inject PrimaryDataStoreDao _storagePoolDao;
     @Inject SnapshotDao _snapshotDao;
     @Inject VirtualMachineManager _itMgr;
-    @Inject DataStoreManager dataStoreMgr;
     @Inject ConfigurationDao _configDao;
     @Inject HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
-    @Inject DiskOfferingDao _diskOfferingDao;
-    @Inject ServiceOfferingDao _serviceOfferingDao;
+    @Inject List<VMSnapshotStrategy> vmSnapshotStrategies;
+
+    public List<VMSnapshotStrategy> getVmSnapshotStrategies() {
+        return vmSnapshotStrategies;
+    }
+
+    @Inject
+    public void setVmSnapshotStrategies(List<VMSnapshotStrategy> vmSnapshotStrategies) {
+        this.vmSnapshotStrategies = vmSnapshotStrategies;
+    }
+
     int _vmSnapshotMax;
     int _wait;
-    StateMachine2<VMSnapshot.State, VMSnapshot.Event, VMSnapshot> _vmSnapshottateMachine ;
+
 
     @Override
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
@@ -144,7 +112,6 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
         String value = _configDao.getValue("vmsnapshot.create.wait");
         _wait = NumbersUtil.parseInt(value, 1800);
 
-        _vmSnapshottateMachine   = VMSnapshot.State.getStateMachine();
         return true;
     }
 
@@ -336,6 +303,22 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
         return _name;
     }
 
+    private VMSnapshotStrategy findVMSnapshotStrategy(VMSnapshot vmSnapshot) {
+        VMSnapshotStrategy snapshotStrategy = null;
+        for(VMSnapshotStrategy strategy : vmSnapshotStrategies) {
+            if (strategy.canHandle(vmSnapshot)) {
+                snapshotStrategy = strategy;
+                break;
+            }
+        }
+
+        if (snapshotStrategy == null) {
+            throw new CloudRuntimeException("can't find vm snapshot strategy for vmsnapshot: " + vmSnapshot.getId());
+        }
+
+        return snapshotStrategy;
+    }
+
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_VM_SNAPSHOT_CREATE, eventDescription = "creating VM snapshot", async = true)
     public VMSnapshot creatVMSnapshot(Long vmId, Long vmSnapshotId) {
@@ -347,241 +330,21 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
         if(vmSnapshot == null){
             throw new CloudRuntimeException("VM snapshot id: " + vmSnapshotId + " can not be found");
         }
-        Long hostId = pickRunningHost(vmId);
-        try {
-            vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.CreateRequested);
-        } catch (NoTransitionException e) {
-            throw new CloudRuntimeException(e.getMessage());
-        }
-        return createVmSnapshotInternal(userVm, vmSnapshot, hostId);
-    }
-
-    protected VMSnapshot createVmSnapshotInternal(UserVmVO userVm, VMSnapshotVO vmSnapshot, Long hostId) {
-        CreateVMSnapshotAnswer answer = null;
-        try {
-            GuestOSVO guestOS = _guestOSDao.findById(userVm.getGuestOSId());
-            
-            // prepare snapshotVolumeTos
-            List<VolumeTO> volumeTOs = getVolumeTOList(userVm.getId());
-            
-            // prepare target snapshotTO and its parent snapshot (current snapshot)
-            VMSnapshotTO current = null;
-            VMSnapshotVO currentSnapshot = _vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId());
-            if (currentSnapshot != null)
-                current = getSnapshotWithParents(currentSnapshot);
-            VMSnapshotTO target = new VMSnapshotTO(vmSnapshot.getId(),  vmSnapshot.getName(), vmSnapshot.getType(), null, vmSnapshot.getDescription(), false,
-                    current);
-            if (current == null)
-                vmSnapshot.setParent(null);
-            else
-                vmSnapshot.setParent(current.getId());
-
-            CreateVMSnapshotCommand ccmd = new CreateVMSnapshotCommand(userVm.getInstanceName(),target ,volumeTOs, guestOS.getDisplayName(),userVm.getState());
-            ccmd.setWait(_wait);
-            
-            answer = (CreateVMSnapshotAnswer) sendToPool(hostId, ccmd);
-            if (answer != null && answer.getResult()) {
-                processAnswer(vmSnapshot, userVm, answer, hostId);
-                s_logger.debug("Create vm snapshot " + vmSnapshot.getName() + " succeeded for vm: " + userVm.getInstanceName());
-            }else{
-                
-                String errMsg = "Creating VM snapshot: " + vmSnapshot.getName() + " failed";
-                if(answer != null && answer.getDetails() != null)
-                    errMsg = errMsg + " due to " + answer.getDetails();
-                s_logger.error(errMsg);
-                vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed);
-                throw new CloudRuntimeException(errMsg);
-            }
-            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);
-        } finally{
-            if(vmSnapshot.getState() == VMSnapshot.State.Allocated){
-                s_logger.warn("Create vm snapshot " + vmSnapshot.getName() + " failed for vm: " + userVm.getInstanceName());
-                _vmSnapshotDao.remove(vmSnapshot.getId());
-            }
-            if(vmSnapshot.getState() == VMSnapshot.State.Ready && answer != null){
-                for (VolumeTO volumeTo : answer.getVolumeTOs()){
-                    publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_CREATE,vmSnapshot,userVm,volumeTo);
-                }
-            }
-        }
-    }
-
-    private void publishUsageEvent(String type, VMSnapshot vmSnapshot, UserVm userVm, VolumeTO volumeTo){
-        VolumeVO volume = _volumeDao.findById(volumeTo.getId());
-        Long diskOfferingId = volume.getDiskOfferingId();
-        Long offeringId = null;
-        if (diskOfferingId != null) {
-            DiskOfferingVO offering = _diskOfferingDao.findById(diskOfferingId);
-            if (offering != null
-                    && (offering.getType() == DiskOfferingVO.Type.Disk)) {
-                offeringId = offering.getId();
-            }
-        }
-        UsageEventUtils.publishUsageEvent(
-                type,
-                vmSnapshot.getAccountId(),
-                userVm.getDataCenterId(),
-                userVm.getId(),
-                vmSnapshot.getName(),
-                offeringId,     
-                volume.getId(), // save volume's id into templateId field
-                volumeTo.getChainSize(),
-                VMSnapshot.class.getName(), vmSnapshot.getUuid());       
-    }
-    
-    protected List<VolumeTO> getVolumeTOList(Long vmId) {
-        List<VolumeTO> volumeTOs = new ArrayList<VolumeTO>();
-        List<VolumeVO> volumeVos = _volumeDao.findByInstance(vmId);
-        
-        for (VolumeVO volume : volumeVos) {
-            StoragePool pool = (StoragePool)dataStoreMgr.getPrimaryDataStore(volume.getPoolId());
-            VolumeTO volumeTO = new VolumeTO(volume, pool);
-            volumeTOs.add(volumeTO);
-        }
-        return volumeTOs;
-    }
-
-    // get snapshot and its parents recursively
-    private VMSnapshotTO getSnapshotWithParents(VMSnapshotVO snapshot) {
-        Map<Long, VMSnapshotVO> snapshotMap = new HashMap<Long, VMSnapshotVO>();
-        List<VMSnapshotVO> allSnapshots = _vmSnapshotDao.findByVm(snapshot.getVmId());
-        for (VMSnapshotVO vmSnapshotVO : allSnapshots) {
-            snapshotMap.put(vmSnapshotVO.getId(), vmSnapshotVO);
-        }
-
-        VMSnapshotTO currentTO = convert2VMSnapshotTO(snapshot);
-        VMSnapshotTO result = currentTO;
-        VMSnapshotVO current = snapshot;
-        while (current.getParent() != null) {
-            VMSnapshotVO parent = snapshotMap.get(current.getParent());
-            currentTO.setParent(convert2VMSnapshotTO(parent));
-            current = snapshotMap.get(current.getParent());
-            currentTO = currentTO.getParent();
-        }
-        return result;
-    }
 
-    private VMSnapshotTO convert2VMSnapshotTO(VMSnapshotVO vo) {
-        return new VMSnapshotTO(vo.getId(), vo.getName(),  vo.getType(), vo.getCreated().getTime(), vo.getDescription(),
-                vo.getCurrent(), null);
-    }
-
-    protected boolean vmSnapshotStateTransitTo(VMSnapshotVO vsnp, VMSnapshot.Event event) throws NoTransitionException {
-        return _vmSnapshottateMachine.transitTo(vsnp, event, null, _vmSnapshotDao);
-    }
-    
-    @DB
-    protected void processAnswer(VMSnapshotVO vmSnapshot, UserVmVO  userVm, Answer as, Long hostId) {
-        final Transaction txn = Transaction.currentTxn();
         try {
-            txn.start();
-            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());
-                _vmSnapshotDao.remove(vmSnapshot.getId());
-            }
-            txn.commit();
+            VMSnapshotStrategy strategy = findVMSnapshotStrategy(vmSnapshot);
+            VMSnapshot snapshot = strategy.takeVMSnapshot(vmSnapshot);
+            return snapshot;
         } catch (Exception e) {
-            String errMsg = "Error while process answer: " + as.getClass() + " due to " + e.getMessage();
-            s_logger.error(errMsg, e);
-            txn.rollback();
-            throw new CloudRuntimeException(errMsg);
-        } finally {
-            txn.close();
-        }
-    }
-
-    protected void finalizeDelete(VMSnapshotVO vmSnapshot, List<VolumeTO> VolumeTOs) {
-        // update volumes path
-        updateVolumePath(VolumeTOs);
-        
-        // update children's parent snapshots
-        List<VMSnapshotVO> children= _vmSnapshotDao.listByParent(vmSnapshot.getId());
-        for (VMSnapshotVO child : children) {
-            child.setParent(vmSnapshot.getParent());
-            _vmSnapshotDao.persist(child);
+            s_logger.debug("Failed to create vm snapshot: " + vmSnapshotId ,e);
+            return null;
         }
-        
-        // update current snapshot
-        VMSnapshotVO current = _vmSnapshotDao.findCurrentSnapshotByVmId(vmSnapshot.getVmId());
-        if(current != null && current.getId() == vmSnapshot.getId() && vmSnapshot.getParent() != null){
-            VMSnapshotVO parent = _vmSnapshotDao.findById(vmSnapshot.getParent());
-            parent.setCurrent(true);
-            _vmSnapshotDao.persist(parent);
-        }
-        vmSnapshot.setCurrent(false);
-        _vmSnapshotDao.persist(vmSnapshot);
-    }
-
-    protected void finalizeCreate(VMSnapshotVO vmSnapshot, List<VolumeTO> VolumeTOs) {
-        // update volumes path
-        updateVolumePath(VolumeTOs);
-        
-        vmSnapshot.setCurrent(true);
-        
-        // change current snapshot
-        if (vmSnapshot.getParent() != null) {
-            VMSnapshotVO previousCurrent = _vmSnapshotDao.findById(vmSnapshot.getParent());
-            previousCurrent.setCurrent(false);
-            _vmSnapshotDao.persist(previousCurrent);
-        }
-        _vmSnapshotDao.persist(vmSnapshot);
-    }
-
-    protected void finalizeRevert(VMSnapshotVO vmSnapshot, List<VolumeTO> volumeToList) {
-        // update volumes path
-        updateVolumePath(volumeToList);
-
-        // update current snapshot, current snapshot is the one reverted to
-        VMSnapshotVO previousCurrent = _vmSnapshotDao.findCurrentSnapshotByVmId(vmSnapshot.getVmId());
-        if(previousCurrent != null){
-            previousCurrent.setCurrent(false);
-            _vmSnapshotDao.persist(previousCurrent);
-        }
-        vmSnapshot.setCurrent(true);
-        _vmSnapshotDao.persist(vmSnapshot);
     }
 
-    private void updateVolumePath(List<VolumeTO> volumeTOs) {
-        for (VolumeTO volume : volumeTOs) {
-            if (volume.getPath() != null) {
-                VolumeVO volumeVO = _volumeDao.findById(volume.getId());
-                volumeVO.setPath(volume.getPath());
-                volumeVO.setVmSnapshotChainSize(volume.getChainSize());
-                _volumeDao.persist(volumeVO);
-            }
-        }
-    }
-    
     public VMSnapshotManagerImpl() {
         
     }
-    
-    protected Answer sendToPool(Long hostId, Command cmd) throws AgentUnavailableException, OperationTimedoutException {
-        long targetHostId = _hvGuruMgr.getGuruProcessedCommandTargetHost(hostId, cmd);
-        Answer answer = _agentMgr.send(targetHostId, cmd);
-        return answer;
-    }
-    
+
     @Override
     public boolean hasActiveVMSnapshotTasks(Long vmId){
         List<VMSnapshotVO> activeVMSnapshots = _vmSnapshotDao.listByInstanceId(vmId,
@@ -617,50 +380,14 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
 
         if(vmSnapshot.getState() == VMSnapshot.State.Allocated){
             return _vmSnapshotDao.remove(vmSnapshot.getId());
-        }else{
-            return deleteSnapshotInternal(vmSnapshot);
-        }
-    }
-
-    @DB
-    protected boolean deleteSnapshotInternal(VMSnapshotVO vmSnapshot) {
-        UserVmVO userVm = _userVMDao.findById(vmSnapshot.getVmId());
-        DeleteVMSnapshotAnswer answer = null;
-        try {
-            vmSnapshotStateTransitTo(vmSnapshot,VMSnapshot.Event.ExpungeRequested);
-            Long hostId = pickRunningHost(vmSnapshot.getVmId());
-
-            // prepare snapshotVolumeTos
-            List<VolumeTO> volumeTOs = getVolumeTOList(vmSnapshot.getVmId());
-            
-            // prepare DeleteVMSnapshotCommand
-            String vmInstanceName = userVm.getInstanceName();
-            VMSnapshotTO parent = getSnapshotWithParents(vmSnapshot).getParent();
-            VMSnapshotTO vmSnapshotTO = new VMSnapshotTO(vmSnapshot.getId(), vmSnapshot.getName(), vmSnapshot.getType(),
-                    vmSnapshot.getCreated().getTime(), vmSnapshot.getDescription(), vmSnapshot.getCurrent(), parent);
-            GuestOSVO guestOS = _guestOSDao.findById(userVm.getGuestOSId());
-            DeleteVMSnapshotCommand deleteSnapshotCommand = new DeleteVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs,guestOS.getDisplayName());
-            
-            answer = (DeleteVMSnapshotAnswer) sendToPool(hostId, deleteSnapshotCommand);
-           
-            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());
+        } else{
+            try {
+                VMSnapshotStrategy strategy = findVMSnapshotStrategy(vmSnapshot);
+                return strategy.deleteVMSnapshot(vmSnapshot);
+            } catch (Exception e) {
+                s_logger.debug("Failed to delete vm snapshot: " + vmSnapshotId, e);
                 return false;
             }
-        } catch (Exception e) {
-            String msg = "Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + e.getMessage();
-            s_logger.error(msg , e);
-            throw new CloudRuntimeException(e.getMessage());
-        } finally{
-            if(answer != null && answer.getResult()){
-                for (VolumeTO volumeTo : answer.getVolumeTOs()){
-                    publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_DELETE,vmSnapshot,userVm,volumeTo);
-                }
-            }
         }
     }
 
@@ -726,108 +453,29 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
                     throw new CloudRuntimeException(e.getMessage());
                 }
             }
-            hostId = pickRunningHost(userVm.getId());
         }
-        
-        if(hostId == null)
-            throw new CloudRuntimeException("Can not find any host to revert snapshot " + vmSnapshotVo.getName());
-        
+
         // check if there are other active VM snapshot tasks
         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());
-        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());
-            String vmInstanceName = userVm.getInstanceName();
-            VMSnapshotTO parent = getSnapshotWithParents(snapshot).getParent();
-            VMSnapshotTO vmSnapshotTO = new VMSnapshotTO(snapshot.getId(), snapshot.getName(), snapshot.getType(), 
-                    snapshot.getCreated().getTime(), snapshot.getDescription(), snapshot.getCurrent(), parent);
-            
-            GuestOSVO guestOS = _guestOSDao.findById(userVm.getGuestOSId());
-            RevertToVMSnapshotCommand revertToSnapshotCommand = new RevertToVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs, guestOS.getDisplayName());
-           
-            RevertToVMSnapshotAnswer answer = (RevertToVMSnapshotAnswer) sendToPool(hostId, revertToSnapshotCommand);
-            if (answer != null && answer.getResult()) {
-                processAnswer(vmSnapshotVo, userVm, answer, 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
-                vmSnapshotStateTransitTo(vmSnapshotVo, VMSnapshot.Event.OperationFailed);
-                throw new CloudRuntimeException(errMsg);
-            }
+            VMSnapshotStrategy strategy = findVMSnapshotStrategy(vmSnapshotVo);
+            strategy.revertVMSnapshot(vmSnapshotVo);
+            return userVm;
         } catch (Exception e) {
-            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());
+            s_logger.debug("Failed to revert vmsnapshot: " + vmSnapshotId, e);
+            return null;
         }
-        return userVm;
     }
 
-
     @Override
     public VMSnapshot getVMSnapshotById(Long id) {
         VMSnapshotVO vmSnapshot = _vmSnapshotDao.findById(id);
         return vmSnapshot;
     }
 
-    protected Long pickRunningHost(Long vmId) {
-        UserVmVO vm = _userVMDao.findById(vmId);
-        // use VM's host if VM is running
-        if(vm.getState() == State.Running)
-            return vm.getHostId();
-        
-        // check if lastHostId is available
-        if(vm.getLastHostId() != null){
-           HostVO lastHost =  _hostDao.findById(vm.getLastHostId());
-           if(lastHost.getStatus() == com.cloud.host.Status.Up && !lastHost.isInMaintenanceStates())
-               return lastHost.getId();
-        }
-        
-        List<VolumeVO> listVolumes = _volumeDao.findByInstance(vmId);
-        if (listVolumes == null || listVolumes.size() == 0) {
-            throw new InvalidParameterValueException("vmInstance has no volumes");
-        }
-        VolumeVO volume = listVolumes.get(0);
-        Long poolId = volume.getPoolId();
-        if (poolId == null) {
-            throw new InvalidParameterValueException("pool id is not found");
-        }
-        StoragePoolVO storagePool = _storagePoolDao.findById(poolId);
-        if (storagePool == null) {
-            throw new InvalidParameterValueException("storage pool is not found");
-        }
-        List<HostVO> listHost = _hostDao.listAllUpAndEnabledNonHAHosts(Host.Type.Routing, storagePool.getClusterId(), storagePool.getPodId(),
-                storagePool.getDataCenterId(), null);
-        if (listHost == null || listHost.size() == 0) {
-            throw new InvalidParameterValueException("no host in up state is found");
-        }
-        return listHost.get(0).getId();
-    }
 
     @Override
     public VirtualMachine getVMBySnapshotId(Long id) {
@@ -851,7 +499,8 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
             VMSnapshotVO target = _vmSnapshotDao.findById(snapshot.getId());
             if(type != null && target.getType() != type)
                 continue;
-            if (!deleteSnapshotInternal(target)) {
+            VMSnapshotStrategy strategy = findVMSnapshotStrategy(target);
+            if (!strategy.deleteVMSnapshot(target)) {
                 result = false;
                 break;
             }
@@ -869,12 +518,13 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
             
             List<VMSnapshotVO> vmSnapshotsInExpungingStates = _vmSnapshotDao.listByInstanceId(vm.getId(), VMSnapshot.State.Expunging, VMSnapshot.State.Reverting, VMSnapshot.State.Creating);
             for (VMSnapshotVO vmSnapshotVO : vmSnapshotsInExpungingStates) {
+                VMSnapshotStrategy strategy = findVMSnapshotStrategy(vmSnapshotVO);
                 if(vmSnapshotVO.getState() == VMSnapshot.State.Expunging){
-                    return deleteSnapshotInternal(vmSnapshotVO);
+                    return strategy.deleteVMSnapshot(vmSnapshotVO);
                 }else if(vmSnapshotVO.getState() == VMSnapshot.State.Creating){
-                    return createVmSnapshotInternal(userVm, vmSnapshotVO, hostId) != null;
+                    return strategy.takeVMSnapshot(vmSnapshotVO) != null;
                 }else if(vmSnapshotVO.getState() == VMSnapshot.State.Reverting){
-                    return revertInternal(userVm, vmSnapshotVO, hostId) != null;
+                    return strategy.revertVMSnapshot(vmSnapshotVO);
                 }
             }
         }catch (Exception e) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/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 055b2b0..da6e7af 100644
--- a/server/test/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
+++ b/server/test/com/cloud/vm/snapshot/VMSnapshotManagerTest.java
@@ -192,13 +192,6 @@ public class VMSnapshotManagerTest {
         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));
-        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/cloudstack/blob/a6ce66e5/setup/db/db/schema-421to430.sql
----------------------------------------------------------------------
diff --git a/setup/db/db/schema-421to430.sql b/setup/db/db/schema-421to430.sql
index 8e4aa93..d355104 100644
--- a/setup/db/db/schema-421to430.sql
+++ b/setup/db/db/schema-421to430.sql
@@ -39,6 +39,15 @@ ALTER TABLE `cloud`.`vm_instance` ADD COLUMN `power_state_update_count` INT DEFA
 ALTER TABLE `cloud`.`vm_instance` ADD COLUMN `power_host` bigint unsigned;
 ALTER TABLE `cloud`.`vm_instance` ADD CONSTRAINT `fk_vm_instance__power_host` FOREIGN KEY (`power_host`) REFERENCES `cloud`.`host`(`id`);
 
+DROP TABLE IF EXISTS `cloud`.`vm_snapshot_details`;
+CREATE TABLE `cloud`.`vm_snapshot_details` (
+  `id` bigint unsigned UNIQUE NOT NULL,
+  `vm_snapshot_id` bigint unsigned NOT NULL,
+  `name` varchar(255) NOT NULL,
+  `value` varchar(255) NOT NULL,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
 CREATE TABLE `cloud`.`vm_work_job` (
   `id` bigint unsigned UNIQUE NOT NULL,
   `step` char(32) NOT NULL COMMENT 'state',


[2/5] git commit: updated refs/heads/master to 465c9ec

Posted by ed...@apache.org.
move a lot of code into vmsnapshot strategy

fix compile

fix compile

add vm_snapshot_details table in db

add vmsnapshot test cases


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

Branch: refs/heads/master
Commit: a6ce66e55a65eb0fbae9ead92de6ceac7a87c531
Parents: 5c5326f
Author: Edison Su <su...@gmail.com>
Authored: Thu Oct 10 18:00:21 2013 -0700
Committer: Edison Su <su...@gmail.com>
Committed: Fri Oct 25 15:09:04 2013 -0700

----------------------------------------------------------------------
 client/tomcatconf/applicationContext.xml.in     |  12 +-
 .../cloud/agent/api/CreateVMSnapshotAnswer.java |  12 +-
 .../agent/api/CreateVMSnapshotCommand.java      |   4 +-
 .../cloud/agent/api/DeleteVMSnapshotAnswer.java |  12 +-
 .../agent/api/DeleteVMSnapshotCommand.java      |   3 +-
 .../agent/api/RevertToVMSnapshotAnswer.java     |  14 +-
 .../agent/api/RevertToVMSnapshotCommand.java    |   3 +-
 .../cloud/agent/api/VMSnapshotBaseCommand.java  |  10 +-
 .../cloudstack/storage/to/VolumeObjectTO.java   |  12 +
 .../api/storage/VMSnapshotStrategy.java         |  28 ++
 .../cloud/vm/snapshot/VMSnapshotDetailsVO.java  |  87 ++++
 .../src/com/cloud/vm/snapshot/VMSnapshotVO.java |   2 +-
 .../vm/snapshot/dao/VMSnapshotDetailsDao.java   |  28 ++
 .../snapshot/dao/VMSnapshotDetailsDaoImpl.java  |  52 ++
 .../motion/AncientDataMotionStrategy.java       |  26 -
 .../vm/snapshot/dao/VmSnapshotDaoTest.java      |  46 ++
 .../storage/test/ChildTestConfiguration.java    |  47 +-
 .../cloudstack/storage/test/SnapshotTest.java   | 100 ++--
 .../test/resources/storageContext.xml           |   2 +
 engine/storage/snapshot/pom.xml                 |  32 ++
 .../storage/snapshot/SnapshotServiceImpl.java   |  44 +-
 .../snapshot/XenserverSnapshotStrategy.java     |  27 +-
 .../vmsnapshot/DefaultVMSnapshotStrategy.java   | 369 ++++++++++++++
 .../storage/vmsnapshot/VMSnapshotHelper.java    |  38 ++
 .../vmsnapshot/VMSnapshotHelperImpl.java        | 148 ++++++
 .../test/src/VMSnapshotStrategyTest.java        | 256 ++++++++++
 .../storage/volume/VolumeServiceImpl.java       |  85 ++--
 .../manager/VmwareStorageManagerImpl.java       |  87 ++--
 .../xen/resource/CitrixResourceBase.java        |  28 +-
 .../storage/snapshot/SnapshotManagerImpl.java   |  97 ----
 .../vm/snapshot/VMSnapshotManagerImpl.java      | 484 +++----------------
 .../vm/snapshot/VMSnapshotManagerTest.java      |   7 -
 setup/db/db/schema-421to430.sql                 |   9 +
 33 files changed, 1398 insertions(+), 813 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/client/tomcatconf/applicationContext.xml.in
----------------------------------------------------------------------
diff --git a/client/tomcatconf/applicationContext.xml.in b/client/tomcatconf/applicationContext.xml.in
index 2a3520b..13ab71e 100644
--- a/client/tomcatconf/applicationContext.xml.in
+++ b/client/tomcatconf/applicationContext.xml.in
@@ -365,6 +365,7 @@
   <bean id="vMReservationDaoImpl" class="org.apache.cloudstack.engine.cloud.entity.api.db.dao.VMReservationDaoImpl" />
   <bean id="vMRootDiskTagDaoImpl" class="org.apache.cloudstack.engine.cloud.entity.api.db.dao.VMRootDiskTagDaoImpl" />
   <bean id="vMSnapshotDaoImpl" class="com.cloud.vm.snapshot.dao.VMSnapshotDaoImpl" />
+  <bean id="vMSnapshotDetailsDaoImpl" class="com.cloud.vm.snapshot.dao.VMSnapshotDetailsDaoImpl" />
   <bean id="vMTemplateDetailsDaoImpl" class="com.cloud.storage.dao.VMTemplateDetailsDaoImpl" />
   <bean id="vMTemplateHostDaoImpl" class="com.cloud.storage.dao.VMTemplateHostDaoImpl" />
   <bean id="vMTemplatePoolDaoImpl" class="com.cloud.storage.dao.VMTemplatePoolDaoImpl" />
@@ -882,8 +883,17 @@
   <bean id="ApplicationLoadBalancerService" class="org.apache.cloudstack.network.lb.ApplicationLoadBalancerManagerImpl" />
   <bean id="InternalLoadBalancerVMManager" class="org.apache.cloudstack.network.lb.InternalLoadBalancerVMManagerImpl" />
 
-  <bean id="vMSnapshotManagerImpl" class="com.cloud.vm.snapshot.VMSnapshotManagerImpl" />
+  <!--VM snapshot Strategies-->
+  <bean id='vmSnapshotHelper' class="org.apache.cloudstack.storage.vmsnapshot.VMSnapshotHelperImpl" />
+  <bean id='defaultVMSnapshotStrategy' class="org.apache.cloudstack.storage.vmsnapshot.DefaultVMSnapshotStrategy"/>
 
+  <bean id="vMSnapshotManagerImpl" class="com.cloud.vm.snapshot.VMSnapshotManagerImpl">
+    <property name="vmSnapshotStrategies">
+      <list>
+        <ref local="defaultVMSnapshotStrategy"/>
+      </list>
+    </property>
+  </bean>
 
 <!--=======================================================================================================-->
 <!--                                                                                                       -->

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/core/src/com/cloud/agent/api/CreateVMSnapshotAnswer.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/CreateVMSnapshotAnswer.java b/core/src/com/cloud/agent/api/CreateVMSnapshotAnswer.java
index f9fb164..8b8e69e 100644
--- a/core/src/com/cloud/agent/api/CreateVMSnapshotAnswer.java
+++ b/core/src/com/cloud/agent/api/CreateVMSnapshotAnswer.java
@@ -17,21 +17,21 @@
 
 package com.cloud.agent.api;
 
-import java.util.List;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
 
-import com.cloud.agent.api.to.VolumeTO;
+import java.util.List;
 
 public class CreateVMSnapshotAnswer extends Answer {
 
-    private List<VolumeTO> volumeTOs;
+    private List<VolumeObjectTO> volumeTOs;
     private VMSnapshotTO vmSnapshotTo;
     
  
-	public List<VolumeTO> getVolumeTOs() {
+	public List<VolumeObjectTO> getVolumeTOs() {
         return volumeTOs;
     }
 
-    public void setVolumeTOs(List<VolumeTO> volumeTOs) {
+    public void setVolumeTOs(List<VolumeObjectTO> volumeTOs) {
         this.volumeTOs = volumeTOs;
     }
 
@@ -53,7 +53,7 @@ public class CreateVMSnapshotAnswer extends Answer {
     }
 
     public CreateVMSnapshotAnswer(CreateVMSnapshotCommand cmd,
-    		VMSnapshotTO vmSnapshotTo, List<VolumeTO> volumeTOs) {
+    		VMSnapshotTO vmSnapshotTo, List<VolumeObjectTO> volumeTOs) {
         super(cmd, true, "");
         this.vmSnapshotTo = vmSnapshotTo;
         this.volumeTOs = volumeTOs;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/core/src/com/cloud/agent/api/CreateVMSnapshotCommand.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/CreateVMSnapshotCommand.java b/core/src/com/cloud/agent/api/CreateVMSnapshotCommand.java
index 478987d..bfbc21d 100644
--- a/core/src/com/cloud/agent/api/CreateVMSnapshotCommand.java
+++ b/core/src/com/cloud/agent/api/CreateVMSnapshotCommand.java
@@ -18,12 +18,14 @@ package com.cloud.agent.api;
 
 import java.util.List;
 
+import com.cloud.agent.api.to.DataTO;
 import com.cloud.agent.api.to.VolumeTO;
 import com.cloud.vm.VirtualMachine;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
 
 public class CreateVMSnapshotCommand extends VMSnapshotBaseCommand {
 
-    public CreateVMSnapshotCommand(String vmName, VMSnapshotTO snapshot, List<VolumeTO> volumeTOs, String guestOSType, VirtualMachine.State vmState) {
+    public CreateVMSnapshotCommand(String vmName, VMSnapshotTO snapshot, List<VolumeObjectTO> volumeTOs, String guestOSType, VirtualMachine.State vmState) {
         super(vmName, snapshot, volumeTOs, guestOSType);
         this.vmState  = vmState;
     }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/core/src/com/cloud/agent/api/DeleteVMSnapshotAnswer.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/DeleteVMSnapshotAnswer.java b/core/src/com/cloud/agent/api/DeleteVMSnapshotAnswer.java
index 8f4ecad..d6ae95c 100644
--- a/core/src/com/cloud/agent/api/DeleteVMSnapshotAnswer.java
+++ b/core/src/com/cloud/agent/api/DeleteVMSnapshotAnswer.java
@@ -16,12 +16,12 @@
 // under the License.
 package com.cloud.agent.api;
 
-import java.util.List;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
 
-import com.cloud.agent.api.to.VolumeTO;
+import java.util.List;
 
 public class DeleteVMSnapshotAnswer extends Answer {
-    private List<VolumeTO> volumeTOs;
+    private List<VolumeObjectTO> volumeTOs;
 
     public DeleteVMSnapshotAnswer() {
     }
@@ -32,16 +32,16 @@ public class DeleteVMSnapshotAnswer extends Answer {
     }
 
     public DeleteVMSnapshotAnswer(DeleteVMSnapshotCommand cmd,
-            List<VolumeTO> volumeTOs) {
+            List<VolumeObjectTO> volumeTOs) {
         super(cmd, true, "");
         this.volumeTOs = volumeTOs;
     }
 
-    public List<VolumeTO> getVolumeTOs() {
+    public List<VolumeObjectTO> getVolumeTOs() {
         return volumeTOs;
     }
 
-    public void setVolumeTOs(List<VolumeTO> volumeTOs) {
+    public void setVolumeTOs(List<VolumeObjectTO> volumeTOs) {
         this.volumeTOs = volumeTOs;
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/core/src/com/cloud/agent/api/DeleteVMSnapshotCommand.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/DeleteVMSnapshotCommand.java b/core/src/com/cloud/agent/api/DeleteVMSnapshotCommand.java
index c213448..1c64a2b 100644
--- a/core/src/com/cloud/agent/api/DeleteVMSnapshotCommand.java
+++ b/core/src/com/cloud/agent/api/DeleteVMSnapshotCommand.java
@@ -19,10 +19,11 @@ package com.cloud.agent.api;
 import java.util.List;
 
 import com.cloud.agent.api.to.VolumeTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
 
 
 public class DeleteVMSnapshotCommand extends VMSnapshotBaseCommand {
-    public DeleteVMSnapshotCommand(String vmName, VMSnapshotTO snapshot, List<VolumeTO> volumeTOs, String guestOSType) {
+    public DeleteVMSnapshotCommand(String vmName, VMSnapshotTO snapshot, List<VolumeObjectTO> volumeTOs, String guestOSType) {
         super( vmName,  snapshot, volumeTOs, guestOSType);
     }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/core/src/com/cloud/agent/api/RevertToVMSnapshotAnswer.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/RevertToVMSnapshotAnswer.java b/core/src/com/cloud/agent/api/RevertToVMSnapshotAnswer.java
index 848ffc0..6170864 100644
--- a/core/src/com/cloud/agent/api/RevertToVMSnapshotAnswer.java
+++ b/core/src/com/cloud/agent/api/RevertToVMSnapshotAnswer.java
@@ -17,14 +17,14 @@
 
 package com.cloud.agent.api;
 
-import java.util.List;
-
-import com.cloud.agent.api.to.VolumeTO;
 import com.cloud.vm.VirtualMachine;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+
+import java.util.List;
 
 public class RevertToVMSnapshotAnswer extends Answer {
 
-    private List<VolumeTO> volumeTOs;
+    private List<VolumeObjectTO> volumeTOs;
     private VirtualMachine.State vmState;
 
     public RevertToVMSnapshotAnswer(RevertToVMSnapshotCommand cmd, boolean result,
@@ -37,7 +37,7 @@ public class RevertToVMSnapshotAnswer extends Answer {
     }
 
     public RevertToVMSnapshotAnswer(RevertToVMSnapshotCommand cmd,
-            List<VolumeTO> volumeTOs,
+            List<VolumeObjectTO> volumeTOs,
             VirtualMachine.State vmState) {
         super(cmd, true, "");
         this.volumeTOs = volumeTOs;
@@ -48,11 +48,11 @@ public class RevertToVMSnapshotAnswer extends Answer {
         return vmState;
     }
 
-    public List<VolumeTO> getVolumeTOs() {
+    public List<VolumeObjectTO> getVolumeTOs() {
         return volumeTOs;
     }
 
-    public void setVolumeTOs(List<VolumeTO> volumeTOs) {
+    public void setVolumeTOs(List<VolumeObjectTO> volumeTOs) {
         this.volumeTOs = volumeTOs;
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/core/src/com/cloud/agent/api/RevertToVMSnapshotCommand.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/RevertToVMSnapshotCommand.java b/core/src/com/cloud/agent/api/RevertToVMSnapshotCommand.java
index 429a186..1e5fd6c 100644
--- a/core/src/com/cloud/agent/api/RevertToVMSnapshotCommand.java
+++ b/core/src/com/cloud/agent/api/RevertToVMSnapshotCommand.java
@@ -19,10 +19,11 @@ package com.cloud.agent.api;
 import java.util.List;
 
 import com.cloud.agent.api.to.VolumeTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
 
 public class RevertToVMSnapshotCommand extends VMSnapshotBaseCommand {
 
-    public RevertToVMSnapshotCommand(String vmName, VMSnapshotTO snapshot, List<VolumeTO> volumeTOs, String guestOSType) {
+    public RevertToVMSnapshotCommand(String vmName, VMSnapshotTO snapshot, List<VolumeObjectTO> volumeTOs, String guestOSType) {
         super(vmName, snapshot, volumeTOs, guestOSType);
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/core/src/com/cloud/agent/api/VMSnapshotBaseCommand.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/agent/api/VMSnapshotBaseCommand.java b/core/src/com/cloud/agent/api/VMSnapshotBaseCommand.java
index 2120f2f..b2c5241 100644
--- a/core/src/com/cloud/agent/api/VMSnapshotBaseCommand.java
+++ b/core/src/com/cloud/agent/api/VMSnapshotBaseCommand.java
@@ -19,27 +19,29 @@ package com.cloud.agent.api;
 
 import java.util.List;
 
+import com.cloud.agent.api.to.DataTO;
 import com.cloud.agent.api.to.VolumeTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
 
 public class VMSnapshotBaseCommand extends Command{
-    protected List<VolumeTO> volumeTOs;
+    protected List<VolumeObjectTO> volumeTOs;
     protected VMSnapshotTO target;
     protected String vmName;
     protected String guestOSType;
     
     
-    public VMSnapshotBaseCommand(String vmName, VMSnapshotTO snapshot, List<VolumeTO> volumeTOs, String guestOSType) {
+    public VMSnapshotBaseCommand(String vmName, VMSnapshotTO snapshot, List<VolumeObjectTO> volumeTOs, String guestOSType) {
         this.vmName = vmName;
         this.target = snapshot;
         this.volumeTOs = volumeTOs;
         this.guestOSType = guestOSType;
     }
     
-    public List<VolumeTO> getVolumeTOs() {
+    public List<VolumeObjectTO> getVolumeTOs() {
         return volumeTOs;
     }
 
-    public void setVolumeTOs(List<VolumeTO> volumeTOs) {
+    public void setVolumeTOs(List<VolumeObjectTO> volumeTOs) {
         this.volumeTOs = volumeTOs;
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/core/src/org/apache/cloudstack/storage/to/VolumeObjectTO.java
----------------------------------------------------------------------
diff --git a/core/src/org/apache/cloudstack/storage/to/VolumeObjectTO.java b/core/src/org/apache/cloudstack/storage/to/VolumeObjectTO.java
index 5685fad..46659a3 100644
--- a/core/src/org/apache/cloudstack/storage/to/VolumeObjectTO.java
+++ b/core/src/org/apache/cloudstack/storage/to/VolumeObjectTO.java
@@ -38,6 +38,8 @@ public class VolumeObjectTO implements DataTO {
     private String chainInfo;
     private Storage.ImageFormat format;
     private long id;
+
+    private Long deviceId;
     private Long bytesReadRate;
     private Long bytesWriteRate;
     private Long iopsReadRate;
@@ -70,6 +72,7 @@ public class VolumeObjectTO implements DataTO {
         this.iopsReadRate = volume.getIopsReadRate();
         this.iopsWriteRate = volume.getIopsWriteRate();
         this.hypervisorType = volume.getHypervisorType();
+        setDeviceId(volume.getDeviceId());
     }
 
     public String getUuid() {
@@ -220,4 +223,13 @@ public class VolumeObjectTO implements DataTO {
         return iopsWriteRate;
     }
 
+    public Long getDeviceId() {
+        return deviceId;
+    }
+
+    public void setDeviceId(Long deviceId) {
+        this.deviceId = deviceId;
+    }
+
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
new file mode 100644
index 0000000..8dd6eca
--- /dev/null
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
@@ -0,0 +1,28 @@
+/*
+ * 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.engine.subsystem.api.storage;
+
+import com.cloud.vm.snapshot.VMSnapshot;
+
+public interface VMSnapshotStrategy {
+    VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot);
+    boolean deleteVMSnapshot(VMSnapshot vmSnapshot);
+    boolean revertVMSnapshot(VMSnapshot vmSnapshot);
+    boolean canHandle(VMSnapshot vmSnapshot);
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/schema/src/com/cloud/vm/snapshot/VMSnapshotDetailsVO.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/com/cloud/vm/snapshot/VMSnapshotDetailsVO.java b/engine/schema/src/com/cloud/vm/snapshot/VMSnapshotDetailsVO.java
new file mode 100644
index 0000000..934dd92
--- /dev/null
+++ b/engine/schema/src/com/cloud/vm/snapshot/VMSnapshotDetailsVO.java
@@ -0,0 +1,87 @@
+/*
+ * 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 com.cloud.vm.snapshot;
+
+import org.apache.cloudstack.api.InternalIdentity;
+
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+import javax.persistence.Table;
+import javax.persistence.TableGenerator;
+
+@Entity
+@Table(name = "vm_snapshot_details")
+public class VMSnapshotDetailsVO implements InternalIdentity {
+    @Id
+    @TableGenerator(name = "vm_snapshot_details_seq", table = "sequence", pkColumnName = "name", valueColumnName = "value", pkColumnValue = "vm_snapshot_details_seq", allocationSize = 1)
+    @GeneratedValue(strategy = GenerationType.TABLE)
+    @Column(name = "id")
+    private long id;
+
+    @Column(name = "vm_snapshot_id")
+    Long vmSnapshotId;
+
+    @Column(name = "name")
+    String name;
+
+    @Column(name = "value")
+    String value;
+
+    public VMSnapshotDetailsVO() {
+
+    }
+
+    public VMSnapshotDetailsVO(Long vmSnapshotId, String name, String value) {
+        this.vmSnapshotId = vmSnapshotId;
+        this.name = name;
+        this.value = value;
+    }
+
+    public Long getVmSnapshotId() {
+        return this.vmSnapshotId;
+    }
+
+    public void setVmSnapshotId(Long vmSnapshotId) {
+        this.vmSnapshotId = vmSnapshotId;
+    }
+
+    public String getName() {
+        return this.name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
+    public String getValue() {
+        return this.value;
+    }
+
+    public void setValue(String value) {
+        this.value = value;
+    }
+
+    @Override
+    public long getId() {
+       return id;
+    }
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/schema/src/com/cloud/vm/snapshot/VMSnapshotVO.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/com/cloud/vm/snapshot/VMSnapshotVO.java b/engine/schema/src/com/cloud/vm/snapshot/VMSnapshotVO.java
index 03d4945..477148c 100644
--- a/engine/schema/src/com/cloud/vm/snapshot/VMSnapshotVO.java
+++ b/engine/schema/src/com/cloud/vm/snapshot/VMSnapshotVO.java
@@ -41,7 +41,7 @@ public class VMSnapshotVO implements VMSnapshot {
     @TableGenerator(name = "vm_snapshots_sq", table = "sequence", pkColumnName = "name", valueColumnName = "value", pkColumnValue = "vm_snapshots_seq", allocationSize = 1)
     @GeneratedValue(strategy = GenerationType.TABLE)
     @Column(name = "id")
-    long id;
+    Long id;
 
     @Column(name = "uuid")
     String uuid = UUID.randomUUID().toString();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDetailsDao.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDetailsDao.java b/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDetailsDao.java
new file mode 100644
index 0000000..e84178c
--- /dev/null
+++ b/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDetailsDao.java
@@ -0,0 +1,28 @@
+/*
+ * 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 com.cloud.vm.snapshot.dao;
+
+import com.cloud.utils.db.GenericDao;
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+
+import java.util.Map;
+
+public interface VMSnapshotDetailsDao extends GenericDao<VMSnapshotDetailsVO, Long> {
+    Map<String, String> getDetails(Long vmSnapshotId);
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDetailsDaoImpl.java
----------------------------------------------------------------------
diff --git a/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDetailsDaoImpl.java b/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDetailsDaoImpl.java
new file mode 100644
index 0000000..b528b39
--- /dev/null
+++ b/engine/schema/src/com/cloud/vm/snapshot/dao/VMSnapshotDetailsDaoImpl.java
@@ -0,0 +1,52 @@
+/*
+ * 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 com.cloud.vm.snapshot.dao;
+
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class VMSnapshotDetailsDaoImpl extends GenericDaoBase<VMSnapshotDetailsVO, Long> implements VMSnapshotDetailsDao {
+    protected final SearchBuilder<VMSnapshotDetailsVO> searchDetails;
+
+    protected VMSnapshotDetailsDaoImpl() {
+        super();
+        searchDetails = createSearchBuilder();
+        searchDetails.and("vmsnapshotId", searchDetails.entity().getVmSnapshotId(), SearchCriteria.Op.EQ);
+        searchDetails.done();
+    }
+    @Override
+    public Map<String, String> getDetails(Long vmSnapshotId) {
+        SearchCriteria<VMSnapshotDetailsVO> sc = searchDetails.create();
+        sc.setParameters("vmsnapshotId", vmSnapshotId);
+
+        List<VMSnapshotDetailsVO> details = listBy(sc);
+        Map<String, String> detailsMap = new HashMap<String, String>();
+        for (VMSnapshotDetailsVO detail : details) {
+            detailsMap.put(detail.getName(), detail.getValue());
+        }
+
+        return detailsMap;
+    }
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
----------------------------------------------------------------------
diff --git a/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
index 5f5f01e..7b5b7fc 100644
--- a/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
+++ b/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
@@ -87,38 +87,12 @@ AncientDataMotionStrategy implements DataMotionStrategy {
     @Inject
     EndPointSelector selector;
     @Inject
-    TemplateManager templateMgr;
-    @Inject
-    VolumeDataStoreDao volumeStoreDao;
-    @Inject
-    HostDao hostDao;
-    @Inject
     ConfigurationDao configDao;
     @Inject
-    StorageManager storageMgr;
-    @Inject
     VolumeDao volDao;
     @Inject
-    VMTemplateDao templateDao;
-    @Inject
-    SnapshotManager snapshotMgr;
-    @Inject
-    SnapshotDao snapshotDao;
-    @Inject
-    SnapshotDataStoreDao _snapshotStoreDao;
-    @Inject
-    PrimaryDataStoreDao primaryDataStoreDao;
-    @Inject
     DataStoreManager dataStoreMgr;
     @Inject
-    TemplateDataStoreDao templateStoreDao;
-    @Inject
-    DiskOfferingDao diskOfferingDao;
-    @Inject
-    VMTemplatePoolDao templatePoolDao;
-    @Inject
-    VolumeOrchestrationService volumeMgr;
-    @Inject
     StorageCacheManager cacheMgr;
     @Inject
     ManagementService _mgmtServer;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/integration-test/test/com/cloud/vm/snapshot/dao/VmSnapshotDaoTest.java
----------------------------------------------------------------------
diff --git a/engine/storage/integration-test/test/com/cloud/vm/snapshot/dao/VmSnapshotDaoTest.java b/engine/storage/integration-test/test/com/cloud/vm/snapshot/dao/VmSnapshotDaoTest.java
new file mode 100644
index 0000000..fc52f89
--- /dev/null
+++ b/engine/storage/integration-test/test/com/cloud/vm/snapshot/dao/VmSnapshotDaoTest.java
@@ -0,0 +1,46 @@
+/*
+ * 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 com.cloud.vm.snapshot.dao;
+
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+import junit.framework.Assert;
+import org.apache.cloudstack.storage.test.CloudStackTestNGBase;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+
+import javax.inject.Inject;
+import java.util.Map;
+
+@RunWith(SpringJUnit4ClassRunner.class)
+@ContextConfiguration(locations = "classpath:/storageContext.xml")
+public class VmSnapshotDaoTest extends CloudStackTestNGBase {
+    @Inject
+    VMSnapshotDetailsDao vmsnapshotDetailsDao;
+
+    @Test
+    public void testVmSnapshotDetails() {
+        VMSnapshotDetailsVO detailsVO = new VMSnapshotDetailsVO(1L, "test", "foo");
+        vmsnapshotDetailsDao.persist(detailsVO);
+        Map<String, String> details = vmsnapshotDetailsDao.getDetails(1L);
+        Assert.assertTrue(details.containsKey("test"));
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/ChildTestConfiguration.java
----------------------------------------------------------------------
diff --git a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/ChildTestConfiguration.java b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/ChildTestConfiguration.java
index d5eea85..228b957 100644
--- a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/ChildTestConfiguration.java
+++ b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/ChildTestConfiguration.java
@@ -16,30 +16,6 @@
 // under the License.
 package org.apache.cloudstack.storage.test;
 
-import java.io.IOException;
-
-import com.cloud.event.ActionEventUtils;
-import com.cloud.event.dao.EventDaoImpl;
-import org.apache.cloudstack.acl.APIChecker;
-import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
-import org.apache.cloudstack.engine.service.api.OrchestrationService;
-import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDaoImpl;
-import org.apache.cloudstack.framework.rpc.RpcProvider;
-import org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl;
-import org.apache.cloudstack.storage.test.ChildTestConfiguration.Library;
-import org.apache.cloudstack.test.utils.SpringUtils;
-
-import org.mockito.Mockito;
-import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.ComponentScan;
-import org.springframework.context.annotation.ComponentScan.Filter;
-import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.FilterType;
-import org.springframework.core.type.classreading.MetadataReader;
-import org.springframework.core.type.classreading.MetadataReaderFactory;
-import org.springframework.core.type.filter.TypeFilter;
-
 import com.cloud.agent.AgentManager;
 import com.cloud.alert.AlertManager;
 import com.cloud.capacity.dao.CapacityDaoImpl;
@@ -55,6 +31,8 @@ import com.cloud.dc.dao.DcDetailsDaoImpl;
 import com.cloud.dc.dao.HostPodDaoImpl;
 import com.cloud.dc.dao.PodVlanDaoImpl;
 import com.cloud.domain.dao.DomainDaoImpl;
+import com.cloud.event.ActionEventUtils;
+import com.cloud.event.dao.EventDaoImpl;
 import com.cloud.host.dao.HostDao;
 import com.cloud.host.dao.HostDaoImpl;
 import com.cloud.host.dao.HostDetailsDaoImpl;
@@ -80,7 +58,6 @@ import com.cloud.storage.dao.VolumeDaoImpl;
 import com.cloud.storage.dao.VolumeHostDaoImpl;
 import com.cloud.storage.download.DownloadMonitorImpl;
 import com.cloud.storage.secondary.SecondaryStorageVmManager;
-import com.cloud.storage.snapshot.SnapshotManager;
 import com.cloud.tags.dao.ResourceTagsDaoImpl;
 import com.cloud.template.TemplateManager;
 import com.cloud.user.AccountManager;
@@ -96,6 +73,26 @@ import com.cloud.vm.dao.UserVmDaoImpl;
 import com.cloud.vm.dao.UserVmDetailsDaoImpl;
 import com.cloud.vm.dao.VMInstanceDaoImpl;
 import com.cloud.vm.snapshot.dao.VMSnapshotDaoImpl;
+import org.apache.cloudstack.acl.APIChecker;
+import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
+import org.apache.cloudstack.engine.service.api.OrchestrationService;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDaoImpl;
+import org.apache.cloudstack.framework.rpc.RpcProvider;
+import org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl;
+import org.apache.cloudstack.storage.test.ChildTestConfiguration.Library;
+import org.apache.cloudstack.test.utils.SpringUtils;
+import org.mockito.Mockito;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.ComponentScan;
+import org.springframework.context.annotation.ComponentScan.Filter;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.FilterType;
+import org.springframework.core.type.classreading.MetadataReader;
+import org.springframework.core.type.classreading.MetadataReaderFactory;
+import org.springframework.core.type.filter.TypeFilter;
+
+import java.io.IOException;
 
 @Configuration
 @ComponentScan(basePackageClasses = { NicDaoImpl.class, VMInstanceDaoImpl.class, VMTemplateHostDaoImpl.class,

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
----------------------------------------------------------------------
diff --git a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
index 81f77d6..36bc912 100644
--- a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
+++ b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
@@ -18,16 +18,42 @@
  */
 package org.apache.cloudstack.storage.test;
 
-import java.net.URI;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.UUID;
-import java.util.concurrent.ExecutionException;
-
-import javax.inject.Inject;
-
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Command;
+import com.cloud.dc.ClusterVO;
+import com.cloud.dc.DataCenter.NetworkType;
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.HostPodVO;
+import com.cloud.dc.dao.ClusterDao;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.dc.dao.HostPodDao;
+import com.cloud.host.Host;
+import com.cloud.host.Host.Type;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.hypervisor.HypervisorGuruManager;
+import com.cloud.org.Cluster.ClusterType;
+import com.cloud.org.Managed.ManagedState;
+import com.cloud.resource.ResourceManager;
+import com.cloud.resource.ResourceState;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.ScopeType;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.Storage;
+import com.cloud.storage.Storage.ImageFormat;
+import com.cloud.storage.Storage.StoragePoolType;
+import com.cloud.storage.Storage.TemplateType;
+import com.cloud.storage.StoragePoolStatus;
+import com.cloud.storage.VMTemplateVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.VMTemplateDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.component.ComponentContext;
 import junit.framework.Assert;
-
 import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -39,13 +65,14 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
 import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
 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.TemplateService.TemplateApiResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult;
 import org.apache.cloudstack.framework.async.AsyncCallFuture;
 import org.apache.cloudstack.storage.LocalHostEndpoint;
@@ -64,41 +91,12 @@ import org.springframework.test.context.ContextConfiguration;
 import org.testng.AssertJUnit;
 import org.testng.annotations.Test;
 
-import com.cloud.agent.AgentManager;
-import com.cloud.agent.api.Command;
-import com.cloud.dc.ClusterVO;
-import com.cloud.dc.DataCenter.NetworkType;
-import com.cloud.dc.DataCenterVO;
-import com.cloud.dc.HostPodVO;
-import com.cloud.dc.dao.ClusterDao;
-import com.cloud.dc.dao.DataCenterDao;
-import com.cloud.dc.dao.HostPodDao;
-import com.cloud.host.Host;
-import com.cloud.host.Host.Type;
-import com.cloud.host.HostVO;
-import com.cloud.host.dao.HostDao;
-import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import com.cloud.hypervisor.HypervisorGuruManager;
-import com.cloud.org.Cluster.ClusterType;
-import com.cloud.org.Managed.ManagedState;
-import com.cloud.resource.ResourceManager;
-import com.cloud.resource.ResourceState;
-import com.cloud.storage.DataStoreRole;
-import com.cloud.storage.ScopeType;
-import com.cloud.storage.Snapshot;
-import com.cloud.storage.SnapshotVO;
-import com.cloud.storage.Storage;
-import com.cloud.storage.Storage.ImageFormat;
-import com.cloud.storage.Storage.StoragePoolType;
-import com.cloud.storage.Storage.TemplateType;
-import com.cloud.storage.StoragePoolStatus;
-import com.cloud.storage.VMTemplateVO;
-import com.cloud.storage.Volume;
-import com.cloud.storage.VolumeVO;
-import com.cloud.storage.dao.SnapshotDao;
-import com.cloud.storage.dao.VMTemplateDao;
-import com.cloud.storage.dao.VolumeDao;
-import com.cloud.utils.component.ComponentContext;
+import javax.inject.Inject;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.ExecutionException;
 
 @ContextConfiguration(locations = { "classpath:/storageContext.xml" })
 public class SnapshotTest extends CloudStackTestNGBase {
@@ -404,7 +402,7 @@ public class SnapshotTest extends CloudStackTestNGBase {
         StrategyPriority.sortStrategies(snapshotStrategies, snapshot);
 
         for (SnapshotStrategy strategy : this.snapshotStrategies) {
-            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
+            if (strategy.canHandle(snapshot) != StrategyPriority.Priority.CANT_HANDLE) {
                 snapshot = strategy.takeSnapshot(snapshot);
                 result = true;
             }
@@ -429,7 +427,7 @@ public class SnapshotTest extends CloudStackTestNGBase {
         StrategyPriority.sortStrategies(snapshotStrategies, newSnapshot);
 
         for (SnapshotStrategy strategy : this.snapshotStrategies) {
-            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
+            if (strategy.canHandle(snapshot) != StrategyPriority.Priority.CANT_HANDLE) {
                 newSnapshot = strategy.takeSnapshot(snapshot);
             }
         }
@@ -437,7 +435,7 @@ public class SnapshotTest extends CloudStackTestNGBase {
 
         // create another snapshot
         for (SnapshotStrategy strategy : this.snapshotStrategies) {
-            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
+            if (strategy.canHandle(snapshot) != StrategyPriority.Priority.CANT_HANDLE) {
                 strategy.deleteSnapshot(newSnapshot.getId());
             }
         }
@@ -454,7 +452,7 @@ public class SnapshotTest extends CloudStackTestNGBase {
         StrategyPriority.sortStrategies(snapshotStrategies, snapshot);
 
         for (SnapshotStrategy strategy : this.snapshotStrategies) {
-            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
+            if (strategy.canHandle(snapshot) != StrategyPriority.Priority.CANT_HANDLE) {
                 snapshot = strategy.takeSnapshot(snapshot);
                 result = true;
             }
@@ -487,7 +485,7 @@ public class SnapshotTest extends CloudStackTestNGBase {
         StrategyPriority.sortStrategies(snapshotStrategies, newSnapshot);
 
         for (SnapshotStrategy strategy : this.snapshotStrategies) {
-            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
+            if (strategy.canHandle(snapshot) != StrategyPriority.Priority.CANT_HANDLE) {
                 newSnapshot = strategy.takeSnapshot(snapshot);
             }
         }
@@ -499,7 +497,7 @@ public class SnapshotTest extends CloudStackTestNGBase {
 
         try {
             for (SnapshotStrategy strategy : this.snapshotStrategies) {
-                if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
+                if (strategy.canHandle(snapshot) != StrategyPriority.Priority.CANT_HANDLE) {
                     boolean res = strategy.deleteSnapshot(newSnapshot.getId());
                     Assert.assertTrue(res);
                 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/integration-test/test/resources/storageContext.xml
----------------------------------------------------------------------
diff --git a/engine/storage/integration-test/test/resources/storageContext.xml b/engine/storage/integration-test/test/resources/storageContext.xml
index 664f1e3..884e813 100644
--- a/engine/storage/integration-test/test/resources/storageContext.xml
+++ b/engine/storage/integration-test/test/resources/storageContext.xml
@@ -85,4 +85,6 @@
   <bean id="AccountGuestVlanMapDaoImpl" class="com.cloud.network.dao.AccountGuestVlanMapDaoImpl" />
   <bean id="StorageCacheReplacementAlgorithm" class="org.apache.cloudstack.storage.cache.manager.StorageCacheReplacementAlgorithmLRU" />
   <bean id="ServiceOfferingDetailsDao" class="com.cloud.service.dao.ServiceOfferingDetailsDaoImpl" />
+  <bean id="vmsnapshotDetailsDao" class="com.cloud.vm.snapshot.dao.VMSnapshotDetailsDaoImpl" />
+  <bean id="snapshotManager" class="com.cloud.storage.snapshot.SnapshotManagerImpl" />
 </beans>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/snapshot/pom.xml
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/pom.xml b/engine/storage/snapshot/pom.xml
index 8a84704..808d0c2 100644
--- a/engine/storage/snapshot/pom.xml
+++ b/engine/storage/snapshot/pom.xml
@@ -30,5 +30,37 @@
       <artifactId>cloud-engine-api</artifactId>
       <version>${project.version}</version>
     </dependency>
+      <dependency>
+          <groupId>org.apache.cloudstack</groupId>
+          <artifactId>cloud-api</artifactId>
+          <version>${project.version}</version>
+          <type>test-jar</type>
+          <scope>test</scope>
+      </dependency>
   </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>testCompile</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+            <plugin>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <phase>integration-test</phase>
+                        <goals>
+                            <goal>test</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
 </project>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java
----------------------------------------------------------------------
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
index a4014b0..0799721 100644
--- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java
@@ -17,11 +17,10 @@
 
 package org.apache.cloudstack.storage.snapshot;
 
-import java.util.concurrent.ExecutionException;
-
-import javax.inject.Inject;
-
-import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Snapshot;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionService;
@@ -41,57 +40,26 @@ import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
 import org.apache.cloudstack.framework.async.AsyncRpcContext;
 import org.apache.cloudstack.storage.command.CommandResult;
 import org.apache.cloudstack.storage.command.CopyCmdAnswer;
-import org.apache.cloudstack.storage.datastore.ObjectInDataStoreManager;
 import org.apache.cloudstack.storage.datastore.PrimaryDataStore;
-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.log4j.Logger;
 import org.springframework.stereotype.Component;
 
-import com.cloud.dc.dao.ClusterDao;
-import com.cloud.storage.DataStoreRole;
-import com.cloud.storage.Snapshot;
-import com.cloud.storage.dao.SnapshotDao;
-import com.cloud.storage.dao.VolumeDao;
-import com.cloud.storage.snapshot.SnapshotManager;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.fsm.NoTransitionException;
-import com.cloud.vm.dao.UserVmDao;
-import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import javax.inject.Inject;
+import java.util.concurrent.ExecutionException;
 
 @Component
 public class SnapshotServiceImpl implements SnapshotService {
     private static final Logger s_logger = Logger.getLogger(SnapshotServiceImpl.class);
     @Inject
-    protected VolumeDao _volsDao;
-    @Inject
-    protected UserVmDao _vmDao;
-    @Inject
-    protected PrimaryDataStoreDao _storagePoolDao;
-    @Inject
-    protected ClusterDao _clusterDao;
-    @Inject
-    protected SnapshotDao _snapshotDao;
-    @Inject
     protected SnapshotDataStoreDao _snapshotStoreDao;
-
-    @Inject
-    protected SnapshotManager snapshotMgr;
-    @Inject
-    protected VolumeOrchestrationService volumeMgr;
-    @Inject
-    protected SnapshotStateMachineManager stateMachineManager;
     @Inject
     SnapshotDataFactory snapshotfactory;
     @Inject
     DataStoreManager dataStoreMgr;
     @Inject
     DataMotionService motionSrv;
-    @Inject
-    ObjectInDataStoreManager objInStoreMgr;
-    @Inject
-    VMSnapshotDao _vmSnapshotDao;
 
     static private class CreateSnapshotContext<T> extends AsyncRpcContext<T> {
         final SnapshotInfo snapshot;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
index 6a874d6..403f113 100644
--- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
@@ -16,8 +16,17 @@
 // under the License.
 package org.apache.cloudstack.storage.snapshot;
 
-import javax.inject.Inject;
-
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.Volume;
+import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.snapshot.SnapshotManager;
+import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
 import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
@@ -36,25 +45,13 @@ import org.apache.cloudstack.storage.to.SnapshotObjectTO;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
-import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.storage.DataStoreRole;
-import com.cloud.storage.Snapshot;
-import com.cloud.storage.SnapshotVO;
-import com.cloud.storage.Volume;
-import com.cloud.storage.dao.SnapshotDao;
-import com.cloud.storage.snapshot.SnapshotManager;
-import com.cloud.utils.NumbersUtil;
-import com.cloud.utils.db.DB;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.fsm.NoTransitionException;
+import javax.inject.Inject;
 
 @Component
 public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
     private static final Logger s_logger = Logger.getLogger(XenserverSnapshotStrategy.class);
 
     @Inject
-    SnapshotManager snapshotMgr;
-    @Inject
     SnapshotService snapshotSvr;
     @Inject
     DataStoreManager dataStoreMgr;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
new file mode 100644
index 0000000..6b5e5fb
--- /dev/null
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
@@ -0,0 +1,369 @@
+/*
+ * 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.vmsnapshot;
+
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.log4j.Logger;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CreateVMSnapshotAnswer;
+import com.cloud.agent.api.CreateVMSnapshotCommand;
+import com.cloud.agent.api.DeleteVMSnapshotAnswer;
+import com.cloud.agent.api.DeleteVMSnapshotCommand;
+import com.cloud.agent.api.RevertToVMSnapshotAnswer;
+import com.cloud.agent.api.RevertToVMSnapshotCommand;
+import com.cloud.agent.api.VMSnapshotTO;
+import com.cloud.event.EventTypes;
+import com.cloud.event.UsageEventUtils;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.storage.DiskOfferingVO;
+import com.cloud.storage.GuestOSVO;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.dao.GuestOSDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.uservm.UserVm;
+import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+
+public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshotStrategy {
+    private static final Logger s_logger = Logger.getLogger(DefaultVMSnapshotStrategy.class);
+    @Inject
+    VMSnapshotHelper vmSnapshotHelper;
+    @Inject
+    GuestOSDao guestOSDao;
+    @Inject
+    UserVmDao userVmDao;
+    @Inject
+    VMSnapshotDao vmSnapshotDao;
+    int _wait;
+    @Inject
+    ConfigurationDao configurationDao;
+    @Inject
+    AgentManager agentMgr;
+    @Inject
+    VolumeDao volumeDao;
+    @Inject
+    DiskOfferingDao diskOfferingDao;
+    @Override
+    public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
+        String value = configurationDao.getValue("vmsnapshot.create.wait");
+        _wait = NumbersUtil.parseInt(value, 1800);
+        return true;
+    }
+
+    public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
+        Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshot.getVmId());
+        UserVm userVm = userVmDao.findById(vmSnapshot.getVmId());
+        VMSnapshotVO vmSnapshotVO = (VMSnapshotVO)vmSnapshot;
+        try {
+            vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshotVO, VMSnapshot.Event.CreateRequested);
+        } catch (NoTransitionException e) {
+            throw new CloudRuntimeException(e.getMessage());
+        }
+
+        CreateVMSnapshotAnswer answer = null;
+        boolean result = false;
+        try {
+            GuestOSVO guestOS = guestOSDao.findById(userVm.getGuestOSId());
+
+            List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(userVm.getId());
+
+            VMSnapshotTO current = null;
+            VMSnapshotVO currentSnapshot = vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId());
+            if (currentSnapshot != null)
+                current = vmSnapshotHelper.getSnapshotWithParents(currentSnapshot);
+            VMSnapshotTO target = new VMSnapshotTO(vmSnapshot.getId(),  vmSnapshot.getName(), vmSnapshot.getType(), null, vmSnapshot.getDescription(), false,
+                    current);
+            if (current == null)
+                vmSnapshotVO.setParent(null);
+            else
+                vmSnapshotVO.setParent(current.getId());
+
+            CreateVMSnapshotCommand ccmd = new CreateVMSnapshotCommand(userVm.getInstanceName(),target ,volumeTOs, guestOS.getDisplayName(),userVm.getState());
+            ccmd.setWait(_wait);
+
+            answer = (CreateVMSnapshotAnswer)agentMgr.send(hostId, ccmd);
+            if (answer != null && answer.getResult()) {
+                processAnswer(vmSnapshotVO, userVm, answer, hostId);
+                s_logger.debug("Create vm snapshot " + vmSnapshot.getName() + " succeeded for vm: " + userVm.getInstanceName());
+                result = true;
+
+                for (VolumeObjectTO volumeTo : answer.getVolumeTOs()){
+                    publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_CREATE,vmSnapshot,userVm,volumeTo);
+                }
+                return vmSnapshot;
+            } else {
+                String errMsg = "Creating VM snapshot: " + vmSnapshot.getName() + " failed";
+                if(answer != null && answer.getDetails() != null)
+                    errMsg = errMsg + " due to " + answer.getDetails();
+                s_logger.error(errMsg);
+                throw new CloudRuntimeException(errMsg);
+            }
+        } catch (OperationTimedoutException e) {
+            s_logger.debug("Creating VM snapshot: " + vmSnapshot.getName() + " failed: " + e.toString());
+            throw new CloudRuntimeException("Creating VM snapshot: " + vmSnapshot.getName() + " failed: " + e.toString());
+        } catch (AgentUnavailableException e) {
+            s_logger.debug("Creating VM snapshot: " + vmSnapshot.getName() + " failed", e);
+            throw new CloudRuntimeException("Creating VM snapshot: " + vmSnapshot.getName() + " failed: " + e.toString());
+        } finally{
+            if (!result) {
+                try {
+                    vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed);
+                } catch (NoTransitionException e1) {
+                    s_logger.error("Cannot set vm snapshot state due to: " + e1.getMessage());
+                }
+            }
+        }
+    }
+
+    @Override
+    public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) {
+        UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId());
+        VMSnapshotVO vmSnapshotVO = (VMSnapshotVO)vmSnapshot;
+        try {
+            vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot,VMSnapshot.Event.ExpungeRequested);
+        } catch (NoTransitionException e) {
+            s_logger.debug("Failed to change vm snapshot state with event ExpungeRequested");
+            throw new CloudRuntimeException("Failed to change vm snapshot state with event ExpungeRequested: " + e.getMessage());
+        }
+
+        try {
+            Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshot.getVmId());
+
+            List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(vmSnapshot.getVmId());
+
+            String vmInstanceName = userVm.getInstanceName();
+            VMSnapshotTO parent = vmSnapshotHelper.getSnapshotWithParents(vmSnapshotVO).getParent();
+            VMSnapshotTO vmSnapshotTO = new VMSnapshotTO(vmSnapshot.getId(), vmSnapshot.getName(), vmSnapshot.getType(),
+                    vmSnapshot.getCreated().getTime(), vmSnapshot.getDescription(), vmSnapshot.getCurrent(), parent);
+            GuestOSVO guestOS = guestOSDao.findById(userVm.getGuestOSId());
+            DeleteVMSnapshotCommand deleteSnapshotCommand = new DeleteVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs,guestOS.getDisplayName());
+
+            Answer answer = agentMgr.send(hostId, deleteSnapshotCommand);
+
+            if (answer != null && answer.getResult()) {
+                DeleteVMSnapshotAnswer deleteVMSnapshotAnswer = (DeleteVMSnapshotAnswer)answer;
+                processAnswer(vmSnapshotVO, userVm, answer, hostId);
+                for (VolumeObjectTO volumeTo : deleteVMSnapshotAnswer.getVolumeTOs()){
+                    publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_DELETE,vmSnapshot,userVm,volumeTo);
+                }
+                return true;
+            } else {
+                String errMsg = (answer == null) ? null : answer.getDetails();
+                s_logger.error("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + errMsg);
+                throw new  CloudRuntimeException("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + errMsg);
+            }
+        } catch (OperationTimedoutException e) {
+            throw new CloudRuntimeException("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + e.getMessage());
+        } catch (AgentUnavailableException e) {
+            throw new CloudRuntimeException("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + e.getMessage());
+        }
+    }
+
+    @DB
+    protected void processAnswer(VMSnapshotVO vmSnapshot, UserVm userVm, Answer as, Long hostId) {
+        final Transaction txn = Transaction.currentTxn();
+        try {
+            txn.start();
+            if (as instanceof CreateVMSnapshotAnswer) {
+                CreateVMSnapshotAnswer answer = (CreateVMSnapshotAnswer) as;
+                finalizeCreate(vmSnapshot, answer.getVolumeTOs());
+                vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded);
+            } else if (as instanceof RevertToVMSnapshotAnswer) {
+                RevertToVMSnapshotAnswer answer = (RevertToVMSnapshotAnswer) as;
+                finalizeRevert(vmSnapshot, answer.getVolumeTOs());
+                vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded);
+            } else if (as instanceof DeleteVMSnapshotAnswer) {
+                DeleteVMSnapshotAnswer answer = (DeleteVMSnapshotAnswer) as;
+                finalizeDelete(vmSnapshot, answer.getVolumeTOs());
+                vmSnapshotDao.remove(vmSnapshot.getId());
+            }
+            txn.commit();
+        } catch (Exception e) {
+            String errMsg = "Error while process answer: " + as.getClass() + " due to " + e.getMessage();
+            s_logger.error(errMsg, e);
+            txn.rollback();
+            throw new CloudRuntimeException(errMsg);
+        } finally {
+            txn.close();
+        }
+    }
+
+    protected void finalizeDelete(VMSnapshotVO vmSnapshot, List<VolumeObjectTO> VolumeTOs) {
+        // update volumes path
+        updateVolumePath(VolumeTOs);
+
+        // update children's parent snapshots
+        List<VMSnapshotVO> children= vmSnapshotDao.listByParent(vmSnapshot.getId());
+        for (VMSnapshotVO child : children) {
+            child.setParent(vmSnapshot.getParent());
+            vmSnapshotDao.persist(child);
+        }
+
+        // update current snapshot
+        VMSnapshotVO current = vmSnapshotDao.findCurrentSnapshotByVmId(vmSnapshot.getVmId());
+        if(current != null && current.getId() == vmSnapshot.getId() && vmSnapshot.getParent() != null){
+            VMSnapshotVO parent = vmSnapshotDao.findById(vmSnapshot.getParent());
+            parent.setCurrent(true);
+            vmSnapshotDao.persist(parent);
+        }
+        vmSnapshot.setCurrent(false);
+        vmSnapshotDao.persist(vmSnapshot);
+    }
+
+    protected void finalizeCreate(VMSnapshotVO vmSnapshot, List<VolumeObjectTO> VolumeTOs) {
+        // update volumes path
+        updateVolumePath(VolumeTOs);
+
+        vmSnapshot.setCurrent(true);
+
+        // change current snapshot
+        if (vmSnapshot.getParent() != null) {
+            VMSnapshotVO previousCurrent = vmSnapshotDao.findById(vmSnapshot.getParent());
+            previousCurrent.setCurrent(false);
+            vmSnapshotDao.persist(previousCurrent);
+        }
+        vmSnapshotDao.persist(vmSnapshot);
+    }
+
+    protected void finalizeRevert(VMSnapshotVO vmSnapshot, List<VolumeObjectTO> volumeToList) {
+        // update volumes path
+        updateVolumePath(volumeToList);
+
+        // update current snapshot, current snapshot is the one reverted to
+        VMSnapshotVO previousCurrent = vmSnapshotDao.findCurrentSnapshotByVmId(vmSnapshot.getVmId());
+        if(previousCurrent != null){
+            previousCurrent.setCurrent(false);
+            vmSnapshotDao.persist(previousCurrent);
+        }
+        vmSnapshot.setCurrent(true);
+        vmSnapshotDao.persist(vmSnapshot);
+    }
+
+    private void updateVolumePath(List<VolumeObjectTO> volumeTOs) {
+        for (VolumeObjectTO volume : volumeTOs) {
+            if (volume.getPath() != null) {
+                VolumeVO volumeVO = volumeDao.findById(volume.getId());
+                volumeVO.setPath(volume.getPath());
+                volumeVO.setVmSnapshotChainSize(volume.getSize());
+                volumeDao.persist(volumeVO);
+            }
+        }
+    }
+
+    private void publishUsageEvent(String type, VMSnapshot vmSnapshot, UserVm userVm, VolumeObjectTO volumeTo){
+        VolumeVO volume = volumeDao.findById(volumeTo.getId());
+        Long diskOfferingId = volume.getDiskOfferingId();
+        Long offeringId = null;
+        if (diskOfferingId != null) {
+            DiskOfferingVO offering = diskOfferingDao.findById(diskOfferingId);
+            if (offering != null
+                    && (offering.getType() == DiskOfferingVO.Type.Disk)) {
+                offeringId = offering.getId();
+            }
+        }
+        UsageEventUtils.publishUsageEvent(
+                type,
+                vmSnapshot.getAccountId(),
+                userVm.getDataCenterId(),
+                userVm.getId(),
+                vmSnapshot.getName(),
+                offeringId,
+                volume.getId(), // save volume's id into templateId field
+                volumeTo.getSize(),
+                VMSnapshot.class.getName(), vmSnapshot.getUuid());
+    }
+
+    @Override
+    public boolean revertVMSnapshot(VMSnapshot vmSnapshot) {
+        VMSnapshotVO vmSnapshotVO = (VMSnapshotVO)vmSnapshot;
+        UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId());
+        try {
+            vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshotVO, VMSnapshot.Event.RevertRequested);
+        } catch (NoTransitionException e) {
+            throw new CloudRuntimeException(e.getMessage());
+        }
+
+        boolean result = false;
+        try {
+            VMSnapshotVO snapshot = vmSnapshotDao.findById(vmSnapshotVO.getId());
+            List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(userVm.getId());
+            String vmInstanceName = userVm.getInstanceName();
+            VMSnapshotTO parent = vmSnapshotHelper.getSnapshotWithParents(snapshot).getParent();
+
+            VMSnapshotTO vmSnapshotTO = new VMSnapshotTO(snapshot.getId(), snapshot.getName(), snapshot.getType(),
+                    snapshot.getCreated().getTime(), snapshot.getDescription(), snapshot.getCurrent(), parent);
+            Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshot.getVmId());
+            GuestOSVO guestOS = guestOSDao.findById(userVm.getGuestOSId());
+            RevertToVMSnapshotCommand revertToSnapshotCommand = new RevertToVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs, guestOS.getDisplayName());
+
+            RevertToVMSnapshotAnswer answer = (RevertToVMSnapshotAnswer) agentMgr.send(hostId, revertToSnapshotCommand);
+            if (answer != null && answer.getResult()) {
+                processAnswer(vmSnapshotVO, userVm, answer, hostId);
+                result = true;
+            } 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);
+                throw new CloudRuntimeException(errMsg);
+            }
+        } catch (OperationTimedoutException e) {
+            s_logger.debug("Failed to revert vm snapshot", e);
+            throw new CloudRuntimeException(e.getMessage());
+        } catch (AgentUnavailableException e) {
+            s_logger.debug("Failed to revert vm snapshot", e);
+            throw new CloudRuntimeException(e.getMessage());
+        } finally {
+            if (!result) {
+                try {
+                    vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed);
+                } catch (NoTransitionException e1) {
+                    s_logger.error("Cannot set vm snapshot state due to: " + e1.getMessage());
+                }
+            }
+        }
+        return result;
+    }
+
+    @Override
+    public boolean canHandle(VMSnapshot vmSnapshot) {
+        return true;
+    }
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotHelper.java
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotHelper.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotHelper.java
new file mode 100644
index 0000000..1437f80
--- /dev/null
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotHelper.java
@@ -0,0 +1,38 @@
+/*
+ * 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.vmsnapshot;
+
+import com.cloud.agent.api.VMSnapshotTO;
+import com.cloud.agent.api.to.DataTO;
+import com.cloud.utils.fsm.NoTransitionException;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+
+import java.util.List;
+
+public interface VMSnapshotHelper {
+  boolean vmSnapshotStateTransitTo(VMSnapshot vsnp, VMSnapshot.Event event) throws NoTransitionException;
+
+    Long pickRunningHost(Long vmId);
+
+    List<VolumeObjectTO> getVolumeTOList(Long vmId);
+
+    VMSnapshotTO getSnapshotWithParents(VMSnapshotVO snapshot);
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a6ce66e5/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotHelperImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotHelperImpl.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotHelperImpl.java
new file mode 100644
index 0000000..320a59c
--- /dev/null
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotHelperImpl.java
@@ -0,0 +1,148 @@
+/*
+ * 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.vmsnapshot;
+
+import com.cloud.agent.api.VMSnapshotTO;
+import com.cloud.agent.api.to.DataTO;
+import com.cloud.agent.api.to.VolumeTO;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.VolumeVO;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.utils.fsm.NoTransitionException;
+import com.cloud.utils.fsm.StateMachine2;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+
+import javax.inject.Inject;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class VMSnapshotHelperImpl implements VMSnapshotHelper {
+    @Inject
+    VMSnapshotDao _vmSnapshotDao;
+    @Inject
+    UserVmDao userVmDao;
+    @Inject
+    HostDao hostDao;
+    @Inject
+    VolumeDao volumeDao;
+    @Inject
+    PrimaryDataStoreDao primaryDataStoreDao;
+    @Inject
+    VolumeDataFactory volumeDataFactory;
+
+    StateMachine2<VMSnapshot.State, VMSnapshot.Event, VMSnapshot> _vmSnapshottateMachine ;
+    public VMSnapshotHelperImpl() {
+        _vmSnapshottateMachine   = VMSnapshot.State.getStateMachine();
+    }
+    @Override
+    public boolean vmSnapshotStateTransitTo(VMSnapshot vsnp, VMSnapshot.Event event) throws NoTransitionException {
+        return _vmSnapshottateMachine.transitTo(vsnp, event, null, _vmSnapshotDao);
+    }
+
+    @Override
+    public Long pickRunningHost(Long vmId) {
+        UserVmVO vm = userVmDao.findById(vmId);
+        // use VM's host if VM is running
+        if(vm.getState() == VirtualMachine.State.Running)
+            return vm.getHostId();
+
+        // check if lastHostId is available
+        if(vm.getLastHostId() != null){
+            HostVO lastHost =  hostDao.findById(vm.getLastHostId());
+            if(lastHost.getStatus() == com.cloud.host.Status.Up && !lastHost.isInMaintenanceStates())
+                return lastHost.getId();
+        }
+
+        List<VolumeVO> listVolumes = volumeDao.findByInstance(vmId);
+        if (listVolumes == null || listVolumes.size() == 0) {
+            throw new InvalidParameterValueException("vmInstance has no volumes");
+        }
+        VolumeVO volume = listVolumes.get(0);
+        Long poolId = volume.getPoolId();
+        if (poolId == null) {
+            throw new InvalidParameterValueException("pool id is not found");
+        }
+        StoragePoolVO storagePool = primaryDataStoreDao.findById(poolId);
+        if (storagePool == null) {
+            throw new InvalidParameterValueException("storage pool is not found");
+        }
+        List<HostVO> listHost = hostDao.listAllUpAndEnabledNonHAHosts(Host.Type.Routing, storagePool.getClusterId(), storagePool.getPodId(),
+                storagePool.getDataCenterId(), null);
+        if (listHost == null || listHost.size() == 0) {
+            throw new InvalidParameterValueException("no host in up state is found");
+        }
+        return listHost.get(0).getId();
+    }
+
+    @Override
+    public List<VolumeObjectTO> getVolumeTOList(Long vmId) {
+        List<VolumeObjectTO> volumeTOs = new ArrayList<VolumeObjectTO>();
+        List<VolumeVO> volumeVos = volumeDao.findByInstance(vmId);
+        VolumeInfo volumeInfo = null;
+        for (VolumeVO volume : volumeVos) {
+            volumeInfo = volumeDataFactory.getVolume(volume.getId());
+
+            volumeTOs.add((VolumeObjectTO)volumeInfo.getTO());
+        }
+        return volumeTOs;
+    }
+
+
+    private VMSnapshotTO convert2VMSnapshotTO(VMSnapshotVO vo) {
+        return new VMSnapshotTO(vo.getId(), vo.getName(),  vo.getType(), vo.getCreated().getTime(), vo.getDescription(),
+                vo.getCurrent(), null);
+    }
+
+    @Override
+    public VMSnapshotTO getSnapshotWithParents(VMSnapshotVO snapshot) {
+        Map<Long, VMSnapshotVO> snapshotMap = new HashMap<Long, VMSnapshotVO>();
+        List<VMSnapshotVO> allSnapshots = _vmSnapshotDao.findByVm(snapshot.getVmId());
+        for (VMSnapshotVO vmSnapshotVO : allSnapshots) {
+            snapshotMap.put(vmSnapshotVO.getId(), vmSnapshotVO);
+        }
+
+        VMSnapshotTO currentTO = convert2VMSnapshotTO(snapshot);
+        VMSnapshotTO result = currentTO;
+        VMSnapshotVO current = snapshot;
+        while (current.getParent() != null) {
+            VMSnapshotVO parent = snapshotMap.get(current.getParent());
+            currentTO.setParent(convert2VMSnapshotTO(parent));
+            current = snapshotMap.get(current.getParent());
+            currentTO = currentTO.getParent();
+        }
+        return result;
+    }
+
+}


[3/5] git commit: updated refs/heads/master to 465c9ec

Posted by ed...@apache.org.
Merge branch 'pluggable_vm_snapshot'

Conflicts:
	client/tomcatconf/applicationContext.xml.in
	engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
	engine/storage/integration-test/test/resources/storageContext.xml
	server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
	server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java


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

Branch: refs/heads/master
Commit: 51a8086cf6d1cdf7db3f89468e210020a179a57c
Parents: 56f1277 a6ce66e
Author: Edison Su <su...@gmail.com>
Authored: Fri Oct 25 16:47:17 2013 -0700
Committer: Edison Su <su...@gmail.com>
Committed: Fri Oct 25 16:47:17 2013 -0700

----------------------------------------------------------------------
 .../cloud/agent/api/CreateVMSnapshotAnswer.java |  12 +-
 .../agent/api/CreateVMSnapshotCommand.java      |   4 +-
 .../cloud/agent/api/DeleteVMSnapshotAnswer.java |  12 +-
 .../agent/api/DeleteVMSnapshotCommand.java      |   3 +-
 .../agent/api/RevertToVMSnapshotAnswer.java     |  14 +-
 .../agent/api/RevertToVMSnapshotCommand.java    |   3 +-
 .../cloud/agent/api/VMSnapshotBaseCommand.java  |  10 +-
 .../cloudstack/storage/to/VolumeObjectTO.java   |  12 +
 .../api/storage/VMSnapshotStrategy.java         |  28 ++
 .../cloud/vm/snapshot/VMSnapshotDetailsVO.java  |  87 ++++
 .../src/com/cloud/vm/snapshot/VMSnapshotVO.java |   2 +-
 .../vm/snapshot/dao/VMSnapshotDetailsDao.java   |  28 ++
 .../snapshot/dao/VMSnapshotDetailsDaoImpl.java  |  52 +++
 .../motion/AncientDataMotionStrategy.java       |  26 --
 .../vm/snapshot/dao/VmSnapshotDaoTest.java      |  46 ++
 .../storage/test/ChildTestConfiguration.java    |  47 +-
 .../cloudstack/storage/test/SnapshotTest.java   |  89 ++--
 .../test/resources/storageContext.xml           |   5 +
 engine/storage/snapshot/pom.xml                 |  32 ++
 .../storage/snapshot/SnapshotServiceImpl.java   |  44 +-
 .../snapshot/XenserverSnapshotStrategy.java     |  27 +-
 .../vmsnapshot/DefaultVMSnapshotStrategy.java   | 370 +++++++++++++++
 .../storage/vmsnapshot/VMSnapshotHelper.java    |  38 ++
 .../vmsnapshot/VMSnapshotHelperImpl.java        | 148 ++++++
 .../test/src/VMSnapshotStrategyTest.java        | 256 +++++++++++
 .../storage/volume/VolumeServiceImpl.java       |  85 ++--
 .../manager/VmwareStorageManagerImpl.java       |  87 ++--
 .../xen/resource/CitrixResourceBase.java        |  28 +-
 .../storage/snapshot/SnapshotManagerImpl.java   |  93 ----
 .../vm/snapshot/VMSnapshotManagerImpl.java      | 455 +++----------------
 .../vm/snapshot/VMSnapshotManagerTest.java      |   7 -
 setup/db/db/schema-421to430.sql                 |   9 +
 32 files changed, 1390 insertions(+), 769 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/ChildTestConfiguration.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
----------------------------------------------------------------------
diff --cc engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
index 1407707,36bc912..550a6bf
--- a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
+++ b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
@@@ -406,10 -399,13 +403,11 @@@ public class SnapshotTest extends Cloud
          SnapshotInfo snapshot = this.snapshotFactory.getSnapshot(snapshotVO.getId(), vol.getDataStore());
          boolean result = false;
  
 -        StrategyPriority.sortStrategies(snapshotStrategies, snapshot);
+ 
 -        for (SnapshotStrategy strategy : this.snapshotStrategies) {
 -            if (strategy.canHandle(snapshot) != StrategyPriority.Priority.CANT_HANDLE) {
 -                snapshot = strategy.takeSnapshot(snapshot);
 -                result = true;
 -            }
 +        SnapshotStrategy snapshotStrategy = storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.TAKE);
 +        if (snapshotStrategy != null) {
 +            snapshot = snapshotStrategy.takeSnapshot(snapshot);
 +            result = true;
          }
  
          AssertJUnit.assertTrue(result);
@@@ -428,9 -424,12 +426,11 @@@
          SnapshotInfo snapshot = this.snapshotFactory.getSnapshot(snapshotVO.getId(), vol.getDataStore());
          SnapshotInfo newSnapshot = null;
  
 -        StrategyPriority.sortStrategies(snapshotStrategies, newSnapshot);
+ 
 -        for (SnapshotStrategy strategy : this.snapshotStrategies) {
 -            if (strategy.canHandle(snapshot) != StrategyPriority.Priority.CANT_HANDLE) {
 -                newSnapshot = strategy.takeSnapshot(snapshot);
 -            }
 +        SnapshotStrategy snapshotStrategy = storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.TAKE);
 +        if (snapshotStrategy != null) {
 +            newSnapshot = snapshotStrategy.takeSnapshot(snapshot);
++
          }
          AssertJUnit.assertNotNull(newSnapshot);
  
@@@ -480,11 -482,13 +480,12 @@@
          SnapshotInfo snapshot = this.snapshotFactory.getSnapshot(snapshotVO.getId(), vol.getDataStore());
          SnapshotInfo newSnapshot = null;
  
 -        StrategyPriority.sortStrategies(snapshotStrategies, newSnapshot);
+ 
 -        for (SnapshotStrategy strategy : this.snapshotStrategies) {
 -            if (strategy.canHandle(snapshot) != StrategyPriority.Priority.CANT_HANDLE) {
 -                newSnapshot = strategy.takeSnapshot(snapshot);
 -            }
 +        SnapshotStrategy snapshotStrategy = storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.TAKE);
 +        if (snapshotStrategy != null) {
 +            newSnapshot = snapshotStrategy.takeSnapshot(snapshot);
          }
 +
          AssertJUnit.assertNotNull(newSnapshot);
  
          LocalHostEndpoint ep = new MockLocalHostEndPoint();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/engine/storage/integration-test/test/resources/storageContext.xml
----------------------------------------------------------------------
diff --cc engine/storage/integration-test/test/resources/storageContext.xml
index e4b8274,884e813..0dcd6a8
--- a/engine/storage/integration-test/test/resources/storageContext.xml
+++ b/engine/storage/integration-test/test/resources/storageContext.xml
@@@ -85,5 -85,6 +85,10 @@@
    <bean id="AccountGuestVlanMapDaoImpl" class="com.cloud.network.dao.AccountGuestVlanMapDaoImpl" />
    <bean id="StorageCacheReplacementAlgorithm" class="org.apache.cloudstack.storage.cache.manager.StorageCacheReplacementAlgorithmLRU" />
    <bean id="ServiceOfferingDetailsDao" class="com.cloud.service.dao.ServiceOfferingDetailsDaoImpl" />
++<<<<<<< HEAD
 +  <bean id="storageStrategyFactoryImpl" class="org.apache.cloudstack.storage.helper.StorageStrategyFactoryImpl" />
++=======
+   <bean id="vmsnapshotDetailsDao" class="com.cloud.vm.snapshot.dao.VMSnapshotDetailsDaoImpl" />
+   <bean id="snapshotManager" class="com.cloud.storage.snapshot.SnapshotManagerImpl" />
++>>>>>>> pluggable_vm_snapshot
  </beans>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
----------------------------------------------------------------------
diff --cc engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
index 0000000,6b5e5fb..0ef8ebb
mode 000000,100644..100644
--- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
@@@ -1,0 -1,369 +1,370 @@@
+ /*
+  * 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.vmsnapshot;
+ 
+ import java.util.List;
+ import java.util.Map;
+ 
+ import javax.inject.Inject;
+ import javax.naming.ConfigurationException;
+ 
+ import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
+ import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+ import org.apache.cloudstack.storage.to.VolumeObjectTO;
+ import org.apache.log4j.Logger;
+ 
+ import com.cloud.agent.AgentManager;
+ import com.cloud.agent.api.Answer;
+ import com.cloud.agent.api.CreateVMSnapshotAnswer;
+ import com.cloud.agent.api.CreateVMSnapshotCommand;
+ import com.cloud.agent.api.DeleteVMSnapshotAnswer;
+ import com.cloud.agent.api.DeleteVMSnapshotCommand;
+ import com.cloud.agent.api.RevertToVMSnapshotAnswer;
+ import com.cloud.agent.api.RevertToVMSnapshotCommand;
+ import com.cloud.agent.api.VMSnapshotTO;
+ import com.cloud.event.EventTypes;
+ import com.cloud.event.UsageEventUtils;
+ import com.cloud.exception.AgentUnavailableException;
+ import com.cloud.exception.OperationTimedoutException;
+ import com.cloud.storage.DiskOfferingVO;
+ import com.cloud.storage.GuestOSVO;
+ import com.cloud.storage.VolumeVO;
+ import com.cloud.storage.dao.DiskOfferingDao;
+ import com.cloud.storage.dao.GuestOSDao;
+ import com.cloud.storage.dao.VolumeDao;
+ import com.cloud.uservm.UserVm;
+ import com.cloud.utils.NumbersUtil;
+ import com.cloud.utils.component.ManagerBase;
+ import com.cloud.utils.db.DB;
+ import com.cloud.utils.db.Transaction;
++import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn;
++import com.cloud.utils.db.TransactionStatus;
+ import com.cloud.utils.exception.CloudRuntimeException;
+ import com.cloud.utils.fsm.NoTransitionException;
+ import com.cloud.vm.UserVmVO;
+ import com.cloud.vm.dao.UserVmDao;
+ import com.cloud.vm.snapshot.VMSnapshot;
+ import com.cloud.vm.snapshot.VMSnapshotVO;
+ import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+ 
+ public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshotStrategy {
+     private static final Logger s_logger = Logger.getLogger(DefaultVMSnapshotStrategy.class);
+     @Inject
+     VMSnapshotHelper vmSnapshotHelper;
+     @Inject
+     GuestOSDao guestOSDao;
+     @Inject
+     UserVmDao userVmDao;
+     @Inject
+     VMSnapshotDao vmSnapshotDao;
+     int _wait;
+     @Inject
+     ConfigurationDao configurationDao;
+     @Inject
+     AgentManager agentMgr;
+     @Inject
+     VolumeDao volumeDao;
+     @Inject
+     DiskOfferingDao diskOfferingDao;
+     @Override
+     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
+         String value = configurationDao.getValue("vmsnapshot.create.wait");
+         _wait = NumbersUtil.parseInt(value, 1800);
+         return true;
+     }
+ 
+     public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
+         Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshot.getVmId());
+         UserVm userVm = userVmDao.findById(vmSnapshot.getVmId());
+         VMSnapshotVO vmSnapshotVO = (VMSnapshotVO)vmSnapshot;
+         try {
+             vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshotVO, VMSnapshot.Event.CreateRequested);
+         } catch (NoTransitionException e) {
+             throw new CloudRuntimeException(e.getMessage());
+         }
+ 
+         CreateVMSnapshotAnswer answer = null;
+         boolean result = false;
+         try {
+             GuestOSVO guestOS = guestOSDao.findById(userVm.getGuestOSId());
+ 
+             List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(userVm.getId());
+ 
+             VMSnapshotTO current = null;
+             VMSnapshotVO currentSnapshot = vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId());
+             if (currentSnapshot != null)
+                 current = vmSnapshotHelper.getSnapshotWithParents(currentSnapshot);
+             VMSnapshotTO target = new VMSnapshotTO(vmSnapshot.getId(),  vmSnapshot.getName(), vmSnapshot.getType(), null, vmSnapshot.getDescription(), false,
+                     current);
+             if (current == null)
+                 vmSnapshotVO.setParent(null);
+             else
+                 vmSnapshotVO.setParent(current.getId());
+ 
+             CreateVMSnapshotCommand ccmd = new CreateVMSnapshotCommand(userVm.getInstanceName(),target ,volumeTOs, guestOS.getDisplayName(),userVm.getState());
+             ccmd.setWait(_wait);
+ 
+             answer = (CreateVMSnapshotAnswer)agentMgr.send(hostId, ccmd);
+             if (answer != null && answer.getResult()) {
+                 processAnswer(vmSnapshotVO, userVm, answer, hostId);
+                 s_logger.debug("Create vm snapshot " + vmSnapshot.getName() + " succeeded for vm: " + userVm.getInstanceName());
+                 result = true;
+ 
+                 for (VolumeObjectTO volumeTo : answer.getVolumeTOs()){
+                     publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_CREATE,vmSnapshot,userVm,volumeTo);
+                 }
+                 return vmSnapshot;
+             } else {
+                 String errMsg = "Creating VM snapshot: " + vmSnapshot.getName() + " failed";
+                 if(answer != null && answer.getDetails() != null)
+                     errMsg = errMsg + " due to " + answer.getDetails();
+                 s_logger.error(errMsg);
+                 throw new CloudRuntimeException(errMsg);
+             }
+         } catch (OperationTimedoutException e) {
+             s_logger.debug("Creating VM snapshot: " + vmSnapshot.getName() + " failed: " + e.toString());
+             throw new CloudRuntimeException("Creating VM snapshot: " + vmSnapshot.getName() + " failed: " + e.toString());
+         } catch (AgentUnavailableException e) {
+             s_logger.debug("Creating VM snapshot: " + vmSnapshot.getName() + " failed", e);
+             throw new CloudRuntimeException("Creating VM snapshot: " + vmSnapshot.getName() + " failed: " + e.toString());
+         } finally{
+             if (!result) {
+                 try {
+                     vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed);
+                 } catch (NoTransitionException e1) {
+                     s_logger.error("Cannot set vm snapshot state due to: " + e1.getMessage());
+                 }
+             }
+         }
+     }
+ 
+     @Override
+     public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) {
+         UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId());
+         VMSnapshotVO vmSnapshotVO = (VMSnapshotVO)vmSnapshot;
+         try {
+             vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot,VMSnapshot.Event.ExpungeRequested);
+         } catch (NoTransitionException e) {
+             s_logger.debug("Failed to change vm snapshot state with event ExpungeRequested");
+             throw new CloudRuntimeException("Failed to change vm snapshot state with event ExpungeRequested: " + e.getMessage());
+         }
+ 
+         try {
+             Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshot.getVmId());
+ 
+             List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(vmSnapshot.getVmId());
+ 
+             String vmInstanceName = userVm.getInstanceName();
+             VMSnapshotTO parent = vmSnapshotHelper.getSnapshotWithParents(vmSnapshotVO).getParent();
+             VMSnapshotTO vmSnapshotTO = new VMSnapshotTO(vmSnapshot.getId(), vmSnapshot.getName(), vmSnapshot.getType(),
+                     vmSnapshot.getCreated().getTime(), vmSnapshot.getDescription(), vmSnapshot.getCurrent(), parent);
+             GuestOSVO guestOS = guestOSDao.findById(userVm.getGuestOSId());
+             DeleteVMSnapshotCommand deleteSnapshotCommand = new DeleteVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs,guestOS.getDisplayName());
+ 
+             Answer answer = agentMgr.send(hostId, deleteSnapshotCommand);
+ 
+             if (answer != null && answer.getResult()) {
+                 DeleteVMSnapshotAnswer deleteVMSnapshotAnswer = (DeleteVMSnapshotAnswer)answer;
+                 processAnswer(vmSnapshotVO, userVm, answer, hostId);
+                 for (VolumeObjectTO volumeTo : deleteVMSnapshotAnswer.getVolumeTOs()){
+                     publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_DELETE,vmSnapshot,userVm,volumeTo);
+                 }
+                 return true;
+             } else {
+                 String errMsg = (answer == null) ? null : answer.getDetails();
+                 s_logger.error("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + errMsg);
+                 throw new  CloudRuntimeException("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + errMsg);
+             }
+         } catch (OperationTimedoutException e) {
+             throw new CloudRuntimeException("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + e.getMessage());
+         } catch (AgentUnavailableException e) {
+             throw new CloudRuntimeException("Delete vm snapshot " + vmSnapshot.getName() + " of vm " + userVm.getInstanceName() + " failed due to " + e.getMessage());
+         }
+     }
+ 
+     @DB
 -    protected void processAnswer(VMSnapshotVO vmSnapshot, UserVm userVm, Answer as, Long hostId) {
 -        final Transaction txn = Transaction.currentTxn();
++    protected void processAnswer(final VMSnapshotVO vmSnapshot, UserVm userVm, final Answer as, Long hostId) {
+         try {
 -            txn.start();
 -            if (as instanceof CreateVMSnapshotAnswer) {
 -                CreateVMSnapshotAnswer answer = (CreateVMSnapshotAnswer) as;
 -                finalizeCreate(vmSnapshot, answer.getVolumeTOs());
 -                vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded);
 -            } else if (as instanceof RevertToVMSnapshotAnswer) {
 -                RevertToVMSnapshotAnswer answer = (RevertToVMSnapshotAnswer) as;
 -                finalizeRevert(vmSnapshot, answer.getVolumeTOs());
 -                vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded);
 -            } else if (as instanceof DeleteVMSnapshotAnswer) {
 -                DeleteVMSnapshotAnswer answer = (DeleteVMSnapshotAnswer) as;
 -                finalizeDelete(vmSnapshot, answer.getVolumeTOs());
 -                vmSnapshotDao.remove(vmSnapshot.getId());
 -            }
 -            txn.commit();
++            Transaction.execute(new TransactionCallbackWithExceptionNoReturn<NoTransitionException>() {
++                @Override
++                public void doInTransactionWithoutResult(TransactionStatus status) throws NoTransitionException {
++                    if (as instanceof CreateVMSnapshotAnswer) {
++                        CreateVMSnapshotAnswer answer = (CreateVMSnapshotAnswer) as;
++                        finalizeCreate(vmSnapshot, answer.getVolumeTOs());
++                        vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded);
++                    } else if (as instanceof RevertToVMSnapshotAnswer) {
++                        RevertToVMSnapshotAnswer answer = (RevertToVMSnapshotAnswer) as;
++                        finalizeRevert(vmSnapshot, answer.getVolumeTOs());
++                        vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded);
++                    } else if (as instanceof DeleteVMSnapshotAnswer) {
++                        DeleteVMSnapshotAnswer answer = (DeleteVMSnapshotAnswer) as;
++                        finalizeDelete(vmSnapshot, answer.getVolumeTOs());
++                        vmSnapshotDao.remove(vmSnapshot.getId());
++                    }
++                }
++            });
+         } catch (Exception e) {
+             String errMsg = "Error while process answer: " + as.getClass() + " due to " + e.getMessage();
+             s_logger.error(errMsg, e);
 -            txn.rollback();
+             throw new CloudRuntimeException(errMsg);
 -        } finally {
 -            txn.close();
+         }
+     }
+ 
+     protected void finalizeDelete(VMSnapshotVO vmSnapshot, List<VolumeObjectTO> VolumeTOs) {
+         // update volumes path
+         updateVolumePath(VolumeTOs);
+ 
+         // update children's parent snapshots
+         List<VMSnapshotVO> children= vmSnapshotDao.listByParent(vmSnapshot.getId());
+         for (VMSnapshotVO child : children) {
+             child.setParent(vmSnapshot.getParent());
+             vmSnapshotDao.persist(child);
+         }
+ 
+         // update current snapshot
+         VMSnapshotVO current = vmSnapshotDao.findCurrentSnapshotByVmId(vmSnapshot.getVmId());
+         if(current != null && current.getId() == vmSnapshot.getId() && vmSnapshot.getParent() != null){
+             VMSnapshotVO parent = vmSnapshotDao.findById(vmSnapshot.getParent());
+             parent.setCurrent(true);
+             vmSnapshotDao.persist(parent);
+         }
+         vmSnapshot.setCurrent(false);
+         vmSnapshotDao.persist(vmSnapshot);
+     }
+ 
+     protected void finalizeCreate(VMSnapshotVO vmSnapshot, List<VolumeObjectTO> VolumeTOs) {
+         // update volumes path
+         updateVolumePath(VolumeTOs);
+ 
+         vmSnapshot.setCurrent(true);
+ 
+         // change current snapshot
+         if (vmSnapshot.getParent() != null) {
+             VMSnapshotVO previousCurrent = vmSnapshotDao.findById(vmSnapshot.getParent());
+             previousCurrent.setCurrent(false);
+             vmSnapshotDao.persist(previousCurrent);
+         }
+         vmSnapshotDao.persist(vmSnapshot);
+     }
+ 
+     protected void finalizeRevert(VMSnapshotVO vmSnapshot, List<VolumeObjectTO> volumeToList) {
+         // update volumes path
+         updateVolumePath(volumeToList);
+ 
+         // update current snapshot, current snapshot is the one reverted to
+         VMSnapshotVO previousCurrent = vmSnapshotDao.findCurrentSnapshotByVmId(vmSnapshot.getVmId());
+         if(previousCurrent != null){
+             previousCurrent.setCurrent(false);
+             vmSnapshotDao.persist(previousCurrent);
+         }
+         vmSnapshot.setCurrent(true);
+         vmSnapshotDao.persist(vmSnapshot);
+     }
+ 
+     private void updateVolumePath(List<VolumeObjectTO> volumeTOs) {
+         for (VolumeObjectTO volume : volumeTOs) {
+             if (volume.getPath() != null) {
+                 VolumeVO volumeVO = volumeDao.findById(volume.getId());
+                 volumeVO.setPath(volume.getPath());
+                 volumeVO.setVmSnapshotChainSize(volume.getSize());
+                 volumeDao.persist(volumeVO);
+             }
+         }
+     }
+ 
+     private void publishUsageEvent(String type, VMSnapshot vmSnapshot, UserVm userVm, VolumeObjectTO volumeTo){
+         VolumeVO volume = volumeDao.findById(volumeTo.getId());
+         Long diskOfferingId = volume.getDiskOfferingId();
+         Long offeringId = null;
+         if (diskOfferingId != null) {
+             DiskOfferingVO offering = diskOfferingDao.findById(diskOfferingId);
+             if (offering != null
+                     && (offering.getType() == DiskOfferingVO.Type.Disk)) {
+                 offeringId = offering.getId();
+             }
+         }
+         UsageEventUtils.publishUsageEvent(
+                 type,
+                 vmSnapshot.getAccountId(),
+                 userVm.getDataCenterId(),
+                 userVm.getId(),
+                 vmSnapshot.getName(),
+                 offeringId,
+                 volume.getId(), // save volume's id into templateId field
+                 volumeTo.getSize(),
+                 VMSnapshot.class.getName(), vmSnapshot.getUuid());
+     }
+ 
+     @Override
+     public boolean revertVMSnapshot(VMSnapshot vmSnapshot) {
+         VMSnapshotVO vmSnapshotVO = (VMSnapshotVO)vmSnapshot;
+         UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId());
+         try {
+             vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshotVO, VMSnapshot.Event.RevertRequested);
+         } catch (NoTransitionException e) {
+             throw new CloudRuntimeException(e.getMessage());
+         }
+ 
+         boolean result = false;
+         try {
+             VMSnapshotVO snapshot = vmSnapshotDao.findById(vmSnapshotVO.getId());
+             List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(userVm.getId());
+             String vmInstanceName = userVm.getInstanceName();
+             VMSnapshotTO parent = vmSnapshotHelper.getSnapshotWithParents(snapshot).getParent();
+ 
+             VMSnapshotTO vmSnapshotTO = new VMSnapshotTO(snapshot.getId(), snapshot.getName(), snapshot.getType(),
+                     snapshot.getCreated().getTime(), snapshot.getDescription(), snapshot.getCurrent(), parent);
+             Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshot.getVmId());
+             GuestOSVO guestOS = guestOSDao.findById(userVm.getGuestOSId());
+             RevertToVMSnapshotCommand revertToSnapshotCommand = new RevertToVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs, guestOS.getDisplayName());
+ 
+             RevertToVMSnapshotAnswer answer = (RevertToVMSnapshotAnswer) agentMgr.send(hostId, revertToSnapshotCommand);
+             if (answer != null && answer.getResult()) {
+                 processAnswer(vmSnapshotVO, userVm, answer, hostId);
+                 result = true;
+             } 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);
+                 throw new CloudRuntimeException(errMsg);
+             }
+         } catch (OperationTimedoutException e) {
+             s_logger.debug("Failed to revert vm snapshot", e);
+             throw new CloudRuntimeException(e.getMessage());
+         } catch (AgentUnavailableException e) {
+             s_logger.debug("Failed to revert vm snapshot", e);
+             throw new CloudRuntimeException(e.getMessage());
+         } finally {
+             if (!result) {
+                 try {
+                     vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed);
+                 } catch (NoTransitionException e1) {
+                     s_logger.error("Cannot set vm snapshot state due to: " + e1.getMessage());
+                 }
+             }
+         }
+         return result;
+     }
+ 
+     @Override
+     public boolean canHandle(VMSnapshot vmSnapshot) {
+         return true;
+     }
+ }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
----------------------------------------------------------------------
diff --cc server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
index cd064f5,7a200ff..3f473bd
--- a/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
+++ b/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@@ -17,40 -17,8 +17,39 @@@
  
  package com.cloud.vm.snapshot;
  
 +import java.util.ArrayList;
 +import java.util.Date;
 +import java.util.HashMap;
 +import java.util.List;
 +import java.util.Map;
 +
 +import javax.ejb.Local;
 +import javax.inject.Inject;
 +import javax.naming.ConfigurationException;
 +
 +import org.apache.log4j.Logger;
 +import org.springframework.stereotype.Component;
 +import org.apache.cloudstack.api.command.user.vmsnapshot.ListVMSnapshotCmd;
 +import org.apache.cloudstack.context.CallContext;
 +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
 +import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 +
 +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.DeleteVMSnapshotAnswer;
 +import com.cloud.agent.api.DeleteVMSnapshotCommand;
 +import com.cloud.agent.api.RevertToVMSnapshotAnswer;
 +import com.cloud.agent.api.RevertToVMSnapshotCommand;
 +import com.cloud.agent.api.VMSnapshotTO;
 +import com.cloud.agent.api.to.VolumeTO;
++
  import com.cloud.event.ActionEvent;
  import com.cloud.event.EventTypes;
- import com.cloud.event.UsageEventUtils;
- import com.cloud.exception.AgentUnavailableException;
  import com.cloud.exception.ConcurrentOperationException;
  import com.cloud.exception.InsufficientCapacityException;
  import com.cloud.exception.InvalidParameterValueException;
@@@ -87,16 -44,7 +75,15 @@@ import com.cloud.utils.component.Manage
  import com.cloud.utils.db.Filter;
  import com.cloud.utils.db.SearchBuilder;
  import com.cloud.utils.db.SearchCriteria;
++
 +import com.cloud.utils.db.Transaction;
 +import com.cloud.utils.db.TransactionCallback;
 +import com.cloud.utils.db.TransactionCallbackNoReturn;
 +import com.cloud.utils.db.TransactionCallbackWithException;
 +import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn;
 +import com.cloud.utils.db.TransactionStatus;
++
  import com.cloud.utils.exception.CloudRuntimeException;
- import com.cloud.utils.fsm.NoTransitionException;
- import com.cloud.utils.fsm.StateMachine2;
  import com.cloud.vm.UserVmVO;
  import com.cloud.vm.VMInstanceVO;
  import com.cloud.vm.VirtualMachine;
@@@ -351,165 -330,14 +369,13 @@@ public class VMSnapshotManagerImpl exte
          if(vmSnapshot == null){
              throw new CloudRuntimeException("VM snapshot id: " + vmSnapshotId + " can not be found");
          }
-         Long hostId = pickRunningHost(vmId);
-         try {
-             vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.CreateRequested);
-         } catch (NoTransitionException e) {
-             throw new CloudRuntimeException(e.getMessage());
-         }
-         return createVmSnapshotInternal(userVm, vmSnapshot, hostId);
-     }
--
-     protected VMSnapshot createVmSnapshotInternal(UserVmVO userVm, VMSnapshotVO vmSnapshot, Long hostId) {
-         CreateVMSnapshotAnswer answer = null;
-         try {
-             GuestOSVO guestOS = _guestOSDao.findById(userVm.getGuestOSId());
-             
-             // prepare snapshotVolumeTos
-             List<VolumeTO> volumeTOs = getVolumeTOList(userVm.getId());
-             
-             // prepare target snapshotTO and its parent snapshot (current snapshot)
-             VMSnapshotTO current = null;
-             VMSnapshotVO currentSnapshot = _vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId());
-             if (currentSnapshot != null)
-                 current = getSnapshotWithParents(currentSnapshot);
-             VMSnapshotTO target = new VMSnapshotTO(vmSnapshot.getId(),  vmSnapshot.getName(), vmSnapshot.getType(), null, vmSnapshot.getDescription(), false,
-                     current);
-             if (current == null)
-                 vmSnapshot.setParent(null);
-             else
-                 vmSnapshot.setParent(current.getId());
- 
-             CreateVMSnapshotCommand ccmd = new CreateVMSnapshotCommand(userVm.getInstanceName(),target ,volumeTOs, guestOS.getDisplayName(),userVm.getState());
-             ccmd.setWait(_wait);
-             
-             answer = (CreateVMSnapshotAnswer) sendToPool(hostId, ccmd);
-             if (answer != null && answer.getResult()) {
-                 processAnswer(vmSnapshot, userVm, answer, hostId);
-                 s_logger.debug("Create vm snapshot " + vmSnapshot.getName() + " succeeded for vm: " + userVm.getInstanceName());
-             }else{
-                 
-                 String errMsg = "Creating VM snapshot: " + vmSnapshot.getName() + " failed";
-                 if(answer != null && answer.getDetails() != null)
-                     errMsg = errMsg + " due to " + answer.getDetails();
-                 s_logger.error(errMsg);
-                 vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed);
-                 throw new CloudRuntimeException(errMsg);
-             }
-             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);
-         } finally{
-             if(vmSnapshot.getState() == VMSnapshot.State.Allocated){
-                 s_logger.warn("Create vm snapshot " + vmSnapshot.getName() + " failed for vm: " + userVm.getInstanceName());
-                 _vmSnapshotDao.remove(vmSnapshot.getId());
-             }
-             if(vmSnapshot.getState() == VMSnapshot.State.Ready && answer != null){
-                 for (VolumeTO volumeTo : answer.getVolumeTOs()){
-                     publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_CREATE,vmSnapshot,userVm,volumeTo);
-                 }
-             }
-         }
-     }
- 
-     private void publishUsageEvent(String type, VMSnapshot vmSnapshot, UserVm userVm, VolumeTO volumeTo){
-         VolumeVO volume = _volumeDao.findById(volumeTo.getId());
-         Long diskOfferingId = volume.getDiskOfferingId();
-         Long offeringId = null;
-         if (diskOfferingId != null) {
-             DiskOfferingVO offering = _diskOfferingDao.findById(diskOfferingId);
-             if (offering != null
-                     && (offering.getType() == DiskOfferingVO.Type.Disk)) {
-                 offeringId = offering.getId();
-             }
-         }
-         UsageEventUtils.publishUsageEvent(
-                 type,
-                 vmSnapshot.getAccountId(),
-                 userVm.getDataCenterId(),
-                 userVm.getId(),
-                 vmSnapshot.getName(),
-                 offeringId,     
-                 volume.getId(), // save volume's id into templateId field
-                 volumeTo.getChainSize(),
-                 VMSnapshot.class.getName(), vmSnapshot.getUuid());       
-     }
-     
-     protected List<VolumeTO> getVolumeTOList(Long vmId) {
-         List<VolumeTO> volumeTOs = new ArrayList<VolumeTO>();
-         List<VolumeVO> volumeVos = _volumeDao.findByInstance(vmId);
-         
-         for (VolumeVO volume : volumeVos) {
-             StoragePool pool = (StoragePool)dataStoreMgr.getPrimaryDataStore(volume.getPoolId());
-             VolumeTO volumeTO = new VolumeTO(volume, pool);
-             volumeTOs.add(volumeTO);
-         }
-         return volumeTOs;
-     }
- 
-     // get snapshot and its parents recursively
-     private VMSnapshotTO getSnapshotWithParents(VMSnapshotVO snapshot) {
-         Map<Long, VMSnapshotVO> snapshotMap = new HashMap<Long, VMSnapshotVO>();
-         List<VMSnapshotVO> allSnapshots = _vmSnapshotDao.findByVm(snapshot.getVmId());
-         for (VMSnapshotVO vmSnapshotVO : allSnapshots) {
-             snapshotMap.put(vmSnapshotVO.getId(), vmSnapshotVO);
-         }
- 
-         VMSnapshotTO currentTO = convert2VMSnapshotTO(snapshot);
-         VMSnapshotTO result = currentTO;
-         VMSnapshotVO current = snapshot;
-         while (current.getParent() != null) {
-             VMSnapshotVO parent = snapshotMap.get(current.getParent());
-             currentTO.setParent(convert2VMSnapshotTO(parent));
-             current = snapshotMap.get(current.getParent());
-             currentTO = currentTO.getParent();
-         }
-         return result;
-     }
- 
-     private VMSnapshotTO convert2VMSnapshotTO(VMSnapshotVO vo) {
-         return new VMSnapshotTO(vo.getId(), vo.getName(),  vo.getType(), vo.getCreated().getTime(), vo.getDescription(),
-                 vo.getCurrent(), null);
-     }
- 
-     protected boolean vmSnapshotStateTransitTo(VMSnapshotVO vsnp, VMSnapshot.Event event) throws NoTransitionException {
-         return _vmSnapshottateMachine.transitTo(vsnp, event, null, _vmSnapshotDao);
-     }
-     
-     @DB
-     protected void processAnswer(final VMSnapshotVO vmSnapshot, UserVmVO  userVm, final Answer as, Long hostId) {
          try {
-             Transaction.execute(new TransactionCallbackWithExceptionNoReturn<NoTransitionException>() {
-                 @Override
-                 public void doInTransactionWithoutResult(TransactionStatus status) throws NoTransitionException {
-                     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());
-                         _vmSnapshotDao.remove(vmSnapshot.getId());
-                     }
-                 }
-             });
+             VMSnapshotStrategy strategy = findVMSnapshotStrategy(vmSnapshot);
+             VMSnapshot snapshot = strategy.takeVMSnapshot(vmSnapshot);
+             return snapshot;
          } catch (Exception e) {
-             String errMsg = "Error while process answer: " + as.getClass() + " due to " + e.getMessage();
-             s_logger.error(errMsg, e);
-             throw new CloudRuntimeException(errMsg);
+             s_logger.debug("Failed to create vm snapshot: " + vmSnapshotId ,e);
+             return null;
          }
      }
  

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/51a8086c/setup/db/db/schema-421to430.sql
----------------------------------------------------------------------


[4/5] git commit: updated refs/heads/master to 465c9ec

Posted by ed...@apache.org.
rebase to spring changes


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

Branch: refs/heads/master
Commit: 70b2c0150dad5ca3b42e34d20831a256661bc8f0
Parents: 51a8086
Author: Edison Su <su...@gmail.com>
Authored: Fri Oct 25 18:15:36 2013 -0700
Committer: Edison Su <su...@gmail.com>
Committed: Fri Oct 25 18:15:36 2013 -0700

----------------------------------------------------------------------
 .../core/spring-core-registry-core-context.xml  |  7 ++-
 ...ng-lifecycle-storage-context-inheritable.xml |  6 ++
 .../api/storage/StorageStrategyFactory.java     |  5 +-
 .../api/storage/VMSnapshotStrategy.java         |  2 +-
 .../spring-engine-schema-core-daos-context.xml  |  1 +
 .../core/spring-engine-storage-core-context.xml |  1 +
 ...-engine-storage-snapshot-storage-context.xml |  7 ++-
 .../vmsnapshot/DefaultVMSnapshotStrategy.java   |  5 +-
 .../helper/StorageStrategyFactoryImpl.java      | 22 +++++++
 .../vm/snapshot/VMSnapshotManagerImpl.java      | 62 ++------------------
 10 files changed, 56 insertions(+), 62 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml
----------------------------------------------------------------------
diff --git a/core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml b/core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml
index 26d70c9..a8b2e29 100644
--- a/core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml
+++ b/core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml
@@ -260,9 +260,14 @@
         <property name="excludeKey" value="snapshot.strategies.exclude" />
     </bean>
 
+    <bean id="vmSnapshotStrategiesRegistry"
+        class="org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry">
+        <property name="excludeKey" value="vmSnapshot.strategies.exclude" />
+    </bean>
+
     <bean id="dataMotionStrategiesRegistry"
         class="org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry">
         <property name="excludeKey" value="data.motion.strategies.exclude" />
     </bean>
     
-</beans>
\ No newline at end of file
+</beans>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/core/resources/META-INF/cloudstack/storage/spring-lifecycle-storage-context-inheritable.xml
----------------------------------------------------------------------
diff --git a/core/resources/META-INF/cloudstack/storage/spring-lifecycle-storage-context-inheritable.xml b/core/resources/META-INF/cloudstack/storage/spring-lifecycle-storage-context-inheritable.xml
index b6eed7d..ad78cad 100644
--- a/core/resources/META-INF/cloudstack/storage/spring-lifecycle-storage-context-inheritable.xml
+++ b/core/resources/META-INF/cloudstack/storage/spring-lifecycle-storage-context-inheritable.xml
@@ -64,6 +64,12 @@
     </bean>
 
     <bean class="org.apache.cloudstack.spring.lifecycle.registry.RegistryLifecycle">
+        <property name="registry" ref="vmSnapshotStrategiesRegistry" />
+        <property name="typeClass"
+            value="org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy" />
+    </bean>
+
+    <bean class="org.apache.cloudstack.spring.lifecycle.registry.RegistryLifecycle">
         <property name="registry" ref="dataMotionStrategiesRegistry" />
         <property name="typeClass"
             value="org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy" />

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
index ac1e311..91bcc1f 100644
--- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
@@ -24,6 +24,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.Snaps
 
 import com.cloud.host.Host;
 import com.cloud.storage.Snapshot;
+import com.cloud.vm.snapshot.VMSnapshot;
 
 public interface StorageStrategyFactory {
 
@@ -31,6 +32,8 @@ public interface StorageStrategyFactory {
 
     DataMotionStrategy getDataMotionStrategy(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost);
 
-    SnapshotStrategy getSnapshotStrategy(Snapshot snapshot, SnapshotOperation op); 
+    SnapshotStrategy getSnapshotStrategy(Snapshot snapshot, SnapshotOperation op);
+
+    VMSnapshotStrategy getVmSnapshotStrategy(VMSnapshot vmSnapshot);
 
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
index 8dd6eca..c2a0ded 100644
--- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
@@ -24,5 +24,5 @@ public interface VMSnapshotStrategy {
     VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot);
     boolean deleteVMSnapshot(VMSnapshot vmSnapshot);
     boolean revertVMSnapshot(VMSnapshot vmSnapshot);
-    boolean canHandle(VMSnapshot vmSnapshot);
+    StrategyPriority canHandle(VMSnapshot vmSnapshot);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml
----------------------------------------------------------------------
diff --git a/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml b/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml
index a70cd00..3fce439 100644
--- a/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml
+++ b/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml
@@ -291,6 +291,7 @@
   <bean id="vMReservationDaoImpl" class="org.apache.cloudstack.engine.cloud.entity.api.db.dao.VMReservationDaoImpl" />
   <bean id="vMRootDiskTagDaoImpl" class="org.apache.cloudstack.engine.cloud.entity.api.db.dao.VMRootDiskTagDaoImpl" />
   <bean id="vMSnapshotDaoImpl" class="com.cloud.vm.snapshot.dao.VMSnapshotDaoImpl" />
+  <bean id="vMSnapshotDetailsDaoImpl" class="com.cloud.vm.snapshot.dao.VMSnapshotDetailsDaoImpl" />
   <bean id="vMTemplateDetailsDaoImpl" class="com.cloud.storage.dao.VMTemplateDetailsDaoImpl" />
   <bean id="vMTemplateHostDaoImpl" class="com.cloud.storage.dao.VMTemplateHostDaoImpl" />
   <bean id="vMTemplatePoolDaoImpl" class="com.cloud.storage.dao.VMTemplatePoolDaoImpl" />

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/engine/storage/resources/META-INF/cloudstack/core/spring-engine-storage-core-context.xml
----------------------------------------------------------------------
diff --git a/engine/storage/resources/META-INF/cloudstack/core/spring-engine-storage-core-context.xml b/engine/storage/resources/META-INF/cloudstack/core/spring-engine-storage-core-context.xml
index 8a78fdd..a6a0c22 100644
--- a/engine/storage/resources/META-INF/cloudstack/core/spring-engine-storage-core-context.xml
+++ b/engine/storage/resources/META-INF/cloudstack/core/spring-engine-storage-core-context.xml
@@ -63,6 +63,7 @@
     <bean id="storageStrategyFactoryImpl" class="org.apache.cloudstack.storage.helper.StorageStrategyFactoryImpl" >
         <property name="dataMotionStrategies" value="#{dataMotionStrategiesRegistry.registered}" />
         <property name="snapshotStrategies" value="#{snapshotStrategiesRegistry.registered}" />
+        <property name="vmSnapshotStrategies" value="#{vmSnapshotStrategiesRegistry.registered}" />
     </bean>
 
 </beans>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/engine/storage/snapshot/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml b/engine/storage/snapshot/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
index 3faa894..d25aeea 100644
--- a/engine/storage/snapshot/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
+++ b/engine/storage/snapshot/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
@@ -30,4 +30,9 @@
     <bean id="xenserverSnapshotStrategy"
         class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
 
-</beans>
\ No newline at end of file
+    <bean id="DefaultVMSnapshotStrategy"
+        class="org.apache.cloudstack.storage.vmsnapshot.DefaultVMSnapshotStrategy" />
+
+    <bean id="VMSnapshotHelperImpl"
+        class="org.apache.cloudstack.storage.vmsnapshot.VMSnapshotHelperImpl" />
+</beans>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
index 0ef8ebb..be3cce9 100644
--- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
@@ -24,6 +24,7 @@ import java.util.Map;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
@@ -364,7 +365,7 @@ public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshot
     }
 
     @Override
-    public boolean canHandle(VMSnapshot vmSnapshot) {
-        return true;
+    public StrategyPriority canHandle(VMSnapshot vmSnapshot) {
+        return StrategyPriority.DEFAULT;
     }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java b/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
index 30812bf..a1d128b 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
@@ -31,15 +31,18 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
 import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
+import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 
 import com.cloud.host.Host;
 import com.cloud.storage.Snapshot;
+import com.cloud.vm.snapshot.VMSnapshot;
 
 public class StorageStrategyFactoryImpl implements StorageStrategyFactory {
 
     List<SnapshotStrategy> snapshotStrategies;
     List<DataMotionStrategy> dataMotionStrategies;
+    List<VMSnapshotStrategy> vmSnapshotStrategies;
 
     @Override
     public DataMotionStrategy getDataMotionStrategy(final DataObject srcData, final DataObject destData) {
@@ -71,6 +74,16 @@ public class StorageStrategyFactoryImpl implements StorageStrategyFactory {
         });
     }
 
+    @Override
+    public VMSnapshotStrategy getVmSnapshotStrategy(final VMSnapshot vmSnapshot) {
+       return bestMatch(vmSnapshotStrategies, new CanHandle<VMSnapshotStrategy>() {
+           @Override
+           public StrategyPriority canHandle(VMSnapshotStrategy strategy) {
+                return strategy.canHandle(vmSnapshot);
+           }
+       });
+    }
+
     private static <T> T bestMatch(Collection<T> collection, final CanHandle<T> canHandle) {
         if (collection.size() == 0)
             return null;
@@ -111,4 +124,13 @@ public class StorageStrategyFactoryImpl implements StorageStrategyFactory {
         this.dataMotionStrategies = dataMotionStrategies;
     }
 
+    @Inject
+    public void setVmSnapshotStrategies(List<VMSnapshotStrategy> vmSnapshotStrategies) {
+        this.vmSnapshotStrategies = vmSnapshotStrategies;
+    }
+
+    public List<VMSnapshotStrategy> getVmSnapshotStrategies() {
+        return vmSnapshotStrategies;
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/70b2c015/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 3f473bd..ee81c82 100644
--- a/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
+++ b/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@ -27,26 +27,13 @@ import javax.ejb.Local;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
 import org.apache.cloudstack.api.command.user.vmsnapshot.ListVMSnapshotCmd;
 import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-
-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.DeleteVMSnapshotAnswer;
-import com.cloud.agent.api.DeleteVMSnapshotCommand;
-import com.cloud.agent.api.RevertToVMSnapshotAnswer;
-import com.cloud.agent.api.RevertToVMSnapshotCommand;
-import com.cloud.agent.api.VMSnapshotTO;
-import com.cloud.agent.api.to.VolumeTO;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
 
 import com.cloud.event.ActionEvent;
 import com.cloud.event.EventTypes;
@@ -75,14 +62,6 @@ import com.cloud.utils.component.ManagerBase;
 import com.cloud.utils.db.Filter;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
-
-import com.cloud.utils.db.Transaction;
-import com.cloud.utils.db.TransactionCallback;
-import com.cloud.utils.db.TransactionCallbackNoReturn;
-import com.cloud.utils.db.TransactionCallbackWithException;
-import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn;
-import com.cloud.utils.db.TransactionStatus;
-
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VMInstanceVO;
@@ -92,21 +71,6 @@ import com.cloud.vm.VirtualMachineManager;
 import com.cloud.vm.VirtualMachineProfile;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
-import org.apache.cloudstack.api.command.user.vmsnapshot.ListVMSnapshotCmd;
-import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
-
-import javax.ejb.Local;
-import javax.inject.Inject;
-import javax.naming.ConfigurationException;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
 
 @Component
 @Local(value = { VMSnapshotManager.class, VMSnapshotService.class })
@@ -123,16 +87,8 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
     @Inject VirtualMachineManager _itMgr;
     @Inject ConfigurationDao _configDao;
     @Inject HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
-    @Inject List<VMSnapshotStrategy> vmSnapshotStrategies;
-
-    public List<VMSnapshotStrategy> getVmSnapshotStrategies() {
-        return vmSnapshotStrategies;
-    }
-
     @Inject
-    public void setVmSnapshotStrategies(List<VMSnapshotStrategy> vmSnapshotStrategies) {
-        this.vmSnapshotStrategies = vmSnapshotStrategies;
-    }
+    StorageStrategyFactory storageStrategyFactory;
 
     int _vmSnapshotMax;
     int _wait;
@@ -343,13 +299,7 @@ public class VMSnapshotManagerImpl extends ManagerBase implements VMSnapshotMana
     }
 
     private VMSnapshotStrategy findVMSnapshotStrategy(VMSnapshot vmSnapshot) {
-        VMSnapshotStrategy snapshotStrategy = null;
-        for(VMSnapshotStrategy strategy : vmSnapshotStrategies) {
-            if (strategy.canHandle(vmSnapshot)) {
-                snapshotStrategy = strategy;
-                break;
-            }
-        }
+        VMSnapshotStrategy snapshotStrategy = storageStrategyFactory.getVmSnapshotStrategy(vmSnapshot);
 
         if (snapshotStrategy == null) {
             throw new CloudRuntimeException("can't find vm snapshot strategy for vmsnapshot: " + vmSnapshot.getId());


[5/5] git commit: updated refs/heads/master to 465c9ec

Posted by ed...@apache.org.
Merge remote-tracking branch 'origin/master'


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

Branch: refs/heads/master
Commit: 465c9ec5c3916009a6f87d1d450a5fe39e57b8fe
Parents: 70b2c01 718bc37
Author: Edison Su <su...@gmail.com>
Authored: Fri Oct 25 18:24:29 2013 -0700
Committer: Edison Su <su...@gmail.com>
Committed: Fri Oct 25 18:24:29 2013 -0700

----------------------------------------------------------------------
 .../org/apache/cloudstack/api/APICommand.java   |  4 +++
 .../acl/StaticRoleBasedAPIAccessChecker.java    | 37 +++++++++++++++++---
 .../spring-server-core-managers-context.xml     |  1 +
 .../system/spring-server-system-context.xml     | 36 +++++++++++++++++++
 .../com/cloud/server/ManagementServerImpl.java  | 18 ++++++++--
 ui/scripts/system.js                            |  1 +
 6 files changed, 89 insertions(+), 8 deletions(-)
----------------------------------------------------------------------