Added categorized sorting to SnapshotStrategy and DataMotionStrategy

engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java

@@ -26,9 +26,9 @@ import com.cloud.agent.api.to.VirtualMachineTO;
 import com.cloud.host.Host;
 
 public interface DataMotionStrategy {
-    boolean canHandle(DataObject srcData, DataObject destData);
+    StrategyPriority.Priority canHandle(DataObject srcData, DataObject destData);
 
-    boolean canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost);
+    StrategyPriority.Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost);
 
     Void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback<CopyCommandResult> callback);
 

engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java

@@ -27,5 +27,5 @@ public interface SnapshotStrategy {
 
     boolean revertSnapshot(Long snapshotId);
 
-    boolean canHandle(Snapshot snapshot);
+    StrategyPriority.Priority canHandle(Snapshot snapshot);
 }

engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriority.java

@@ -0,0 +1,83 @@
+package org.apache.cloudstack.engine.subsystem.api.storage;
+
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Map;
+
+import com.cloud.host.Host;
+import com.cloud.storage.Snapshot;
+
+public class StrategyPriority {
+    public enum Priority {
+        CANT_HANDLE,
+        DEFAULT,
+        HYPERVISOR,
+        PLUGIN,
+        HIGHEST
+    }
+
+    public static void sortStrategies(List<SnapshotStrategy> strategies, Snapshot snapshot) {
+        Collections.sort(strategies, new SnapshotStrategyComparator(snapshot));
+    }
+
+    public static void sortStrategies(List<DataMotionStrategy> strategies, DataObject srcData, DataObject destData) {
+        Collections.sort(strategies, new DataMotionStrategyComparator(srcData, destData));
+    }
+
+    public static void sortStrategies(List<DataMotionStrategy> strategies, Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+        Collections.sort(strategies, new DataMotionStrategyHostComparator(volumeMap, srcHost, destHost));
+    }
+
+    static class SnapshotStrategyComparator implements Comparator<SnapshotStrategy> {
+
+        Snapshot snapshot;
+
+        public SnapshotStrategyComparator(Snapshot snapshot) {
+            this.snapshot = snapshot;
+        }
+
+        @Override
+        public int compare(SnapshotStrategy o1, SnapshotStrategy o2) {
+            int i1 = o1.canHandle(snapshot).ordinal();
+            int i2 = o2.canHandle(snapshot).ordinal();
+            return new Integer(i2).compareTo(new Integer(i1));
+        }
+    }
+
+    static class DataMotionStrategyComparator implements Comparator<DataMotionStrategy> {
+
+        DataObject srcData, destData;
+
+        public DataMotionStrategyComparator(DataObject srcData, DataObject destData) {
+            this.srcData = srcData;
+            this.destData = destData;
+        }
+
+        @Override
+        public int compare(DataMotionStrategy o1, DataMotionStrategy o2) {
+            int i1 = o1.canHandle(srcData, destData).ordinal();
+            int i2 = o2.canHandle(srcData, destData).ordinal();
+            return new Integer(i2).compareTo(new Integer(i1));
+        }
+    }
+
+    static class DataMotionStrategyHostComparator implements Comparator<DataMotionStrategy> {
+
+        Host srcHost, destHost;
+        Map<VolumeInfo, DataStore> volumeMap;
+
+        public DataMotionStrategyHostComparator(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+            this.volumeMap = volumeMap;
+            this.srcHost = srcHost;
+            this.destHost = destHost;
+        }
+
+        @Override
+        public int compare(DataMotionStrategy o1, DataMotionStrategy o2) {
+            int i1 = o1.canHandle(volumeMap, srcHost, destHost).ordinal();
+            int i2 = o2.canHandle(volumeMap, srcHost, destHost).ordinal();
+            return new Integer(i2).compareTo(new Integer(i1));
+        }
+    }
+}

engine/api/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java

@@ -0,0 +1,100 @@
+package org.apache.cloudstack.engine.subsystem.api.storage;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority;
+import org.junit.Test;
+
+import com.cloud.host.Host;
+import com.cloud.storage.Snapshot;
+
+import static org.junit.Assert.assertEquals;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+
+public class StrategyPriorityTest {
+
+    @Test
+    public void testSortSnapshotStrategies() {
+        SnapshotStrategy cantHandleStrategy = mock(SnapshotStrategy.class);
+        SnapshotStrategy defaultStrategy = mock(SnapshotStrategy.class);
+        SnapshotStrategy hyperStrategy = mock(SnapshotStrategy.class);
+        SnapshotStrategy pluginStrategy = mock(SnapshotStrategy.class);
+        SnapshotStrategy highestStrategy = mock(SnapshotStrategy.class);
+
+        doReturn(Priority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(Snapshot.class));
+        doReturn(Priority.DEFAULT).when(defaultStrategy).canHandle(any(Snapshot.class));
+        doReturn(Priority.HYPERVISOR).when(hyperStrategy).canHandle(any(Snapshot.class));
+        doReturn(Priority.PLUGIN).when(pluginStrategy).canHandle(any(Snapshot.class));
+        doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(Snapshot.class));
+
+        List<SnapshotStrategy> strategies = new ArrayList<SnapshotStrategy>(5);
+        strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy));
+
+        StrategyPriority.sortStrategies(strategies, mock(Snapshot.class));
+
+        assertEquals("Highest was not 1st.", highestStrategy, strategies.get(0));
+        assertEquals("Plugin was not 2nd.", pluginStrategy, strategies.get(1));
+        assertEquals("Hypervisor was not 3rd.", hyperStrategy, strategies.get(2));
+        assertEquals("Default was not 4th.", defaultStrategy, strategies.get(3));
+        assertEquals("Can't Handle was not 5th.", cantHandleStrategy, strategies.get(4));
+    }
+
+    @Test
+    public void testSortDataMotionStrategies() {
+        DataMotionStrategy cantHandleStrategy = mock(DataMotionStrategy.class);
+        DataMotionStrategy defaultStrategy = mock(DataMotionStrategy.class);
+        DataMotionStrategy hyperStrategy = mock(DataMotionStrategy.class);
+        DataMotionStrategy pluginStrategy = mock(DataMotionStrategy.class);
+        DataMotionStrategy highestStrategy = mock(DataMotionStrategy.class);
+
+        doReturn(Priority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(DataObject.class), any(DataObject.class));
+        doReturn(Priority.DEFAULT).when(defaultStrategy).canHandle(any(DataObject.class), any(DataObject.class));
+        doReturn(Priority.HYPERVISOR).when(hyperStrategy).canHandle(any(DataObject.class), any(DataObject.class));
+        doReturn(Priority.PLUGIN).when(pluginStrategy).canHandle(any(DataObject.class), any(DataObject.class));
+        doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(DataObject.class), any(DataObject.class));
+
+        List<DataMotionStrategy> strategies = new ArrayList<DataMotionStrategy>(5);
+        strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy));
+
+        StrategyPriority.sortStrategies(strategies, mock(DataObject.class), mock(DataObject.class));
+
+        assertEquals("Highest was not 1st.", highestStrategy, strategies.get(0));
+        assertEquals("Plugin was not 2nd.", pluginStrategy, strategies.get(1));
+        assertEquals("Hypervisor was not 3rd.", hyperStrategy, strategies.get(2));
+        assertEquals("Default was not 4th.", defaultStrategy, strategies.get(3));
+        assertEquals("Can't Handle was not 5th.", cantHandleStrategy, strategies.get(4));
+    }
+
+    @Test
+    @SuppressWarnings("unchecked")
+    public void testSortDataMotionStrategies2() {
+        DataMotionStrategy cantHandleStrategy = mock(DataMotionStrategy.class);
+        DataMotionStrategy defaultStrategy = mock(DataMotionStrategy.class);
+        DataMotionStrategy hyperStrategy = mock(DataMotionStrategy.class);
+        DataMotionStrategy pluginStrategy = mock(DataMotionStrategy.class);
+        DataMotionStrategy highestStrategy = mock(DataMotionStrategy.class);
+
+        doReturn(Priority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
+        doReturn(Priority.DEFAULT).when(defaultStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
+        doReturn(Priority.HYPERVISOR).when(hyperStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
+        doReturn(Priority.PLUGIN).when(pluginStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
+        doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
+
+        List<DataMotionStrategy> strategies = new ArrayList<DataMotionStrategy>(5);
+        strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy));
+
+        StrategyPriority.sortStrategies(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
+
+        assertEquals("Highest was not 1st.", highestStrategy, strategies.get(0));
+        assertEquals("Plugin was not 2nd.", pluginStrategy, strategies.get(1));
+        assertEquals("Hypervisor was not 3rd.", hyperStrategy, strategies.get(2));
+        assertEquals("Default was not 4th.", defaultStrategy, strategies.get(3));
+        assertEquals("Can't Handle was not 5th.", cantHandleStrategy, strategies.get(4));
+    }
+}

engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java

@@ -36,6 +36,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreState
 import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority;
 import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
@@ -123,14 +124,13 @@ AncientDataMotionStrategy implements DataMotionStrategy {
     ManagementService _mgmtServer;
 
     @Override
-    public boolean canHandle(DataObject srcData, DataObject destData) {
+    public Priority canHandle(DataObject srcData, DataObject destData) {
-        // TODO Auto-generated method stub
+        return Priority.DEFAULT;
-        return true;
     }
 
     @Override
-    public boolean canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+    public Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
-        return false;
+        return Priority.CANT_HANDLE;
     }
 
     protected boolean needCacheStorage(DataObject srcData, DataObject destData) {

engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java

@@ -18,21 +18,25 @@
  */
 package org.apache.cloudstack.storage.motion;
 
-import com.cloud.agent.api.to.VirtualMachineTO;
+import java.util.List;
-import com.cloud.host.Host;
+import java.util.Map;
-import com.cloud.utils.exception.CloudRuntimeException;
+
+import javax.inject.Inject;
+
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionService;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy;
 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.StrategyPriority;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
 import org.springframework.stereotype.Component;
 
-import javax.inject.Inject;
+import com.cloud.agent.api.to.VirtualMachineTO;
-import java.util.List;
+import com.cloud.host.Host;
-import java.util.Map;
+import com.cloud.utils.exception.CloudRuntimeException;
 
 @Component
 public class DataMotionServiceImpl implements DataMotionService {
@@ -53,8 +57,10 @@ public class DataMotionServiceImpl implements DataMotionService {
             return;
         }
 
+        StrategyPriority.sortStrategies(strategies, srcData, destData);
+
         for (DataMotionStrategy strategy : strategies) {
-            if (strategy.canHandle(srcData, destData)) {
+            if (strategy.canHandle(srcData, destData) != Priority.CANT_HANDLE) {
                 strategy.copyAsync(srcData, destData, callback);
                 return;
             }
@@ -65,8 +71,11 @@ public class DataMotionServiceImpl implements DataMotionService {
     @Override
     public void copyAsync(Map<VolumeInfo, DataStore> volumeMap, VirtualMachineTO vmTo, Host srcHost, Host destHost,
             AsyncCompletionCallback<CopyCommandResult> callback) {
+
+        StrategyPriority.sortStrategies(strategies, volumeMap, srcHost, destHost);
+
         for (DataMotionStrategy strategy : strategies) {
-            if (strategy.canHandle(volumeMap, srcHost, destHost)) {
+            if (strategy.canHandle(volumeMap, srcHost, destHost) != Priority.CANT_HANDLE) {
                 strategy.copyAsync(volumeMap, vmTo, srcHost, destHost, callback);
                 return;
             }

engine/storage/integration-test/test/org/apache/cloudstack/storage/test/MockStorageMotionStrategy.java

@@ -21,31 +21,30 @@ package org.apache.cloudstack.storage.test;
 import java.util.Map;
 import java.util.UUID;
 
-import com.cloud.agent.api.to.DataObjectType;
-import com.cloud.agent.api.to.DataTO;
-import com.cloud.storage.Storage;
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy;
 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.SnapshotInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
-
-import com.cloud.agent.api.to.VirtualMachineTO;
-import com.cloud.host.Host;
 import org.apache.cloudstack.storage.command.CopyCmdAnswer;
-import org.apache.cloudstack.storage.snapshot.SnapshotObject;
 import org.apache.cloudstack.storage.to.SnapshotObjectTO;
 import org.apache.cloudstack.storage.to.TemplateObjectTO;
 
+import com.cloud.agent.api.to.DataObjectType;
+import com.cloud.agent.api.to.DataTO;
+import com.cloud.agent.api.to.VirtualMachineTO;
+import com.cloud.host.Host;
+import com.cloud.storage.Storage;
+
 public class MockStorageMotionStrategy implements DataMotionStrategy {
 
     boolean success = true;
     @Override
-    public boolean canHandle(DataObject srcData, DataObject destData) {
+    public Priority canHandle(DataObject srcData, DataObject destData) {
-        // TODO Auto-generated method stub
+        return Priority.HIGHEST;
-        return true;
     }
 
     public void makeBackupSnapshotSucceed(boolean success) {
@@ -53,8 +52,8 @@ public class MockStorageMotionStrategy implements DataMotionStrategy {
     }
 
     @Override
-    public boolean canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+    public Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
-        return true;
+        return Priority.HIGHEST;
     }
 
     @Override

engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java

@@ -400,8 +400,11 @@ public class SnapshotTest extends CloudStackTestNGBase {
         SnapshotVO snapshotVO = createSnapshotInDb(vol);
         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)) {
+            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
                 snapshot = strategy.takeSnapshot(snapshot);
                 result = true;
             }
@@ -422,8 +425,11 @@ public class SnapshotTest extends CloudStackTestNGBase {
         SnapshotVO snapshotVO = createSnapshotInDb(vol);
         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)) {
+            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
                 newSnapshot = strategy.takeSnapshot(snapshot);
             }
         }
@@ -431,7 +437,7 @@ public class SnapshotTest extends CloudStackTestNGBase {
 
         // create another snapshot
         for (SnapshotStrategy strategy : this.snapshotStrategies) {
-            if (strategy.canHandle(snapshot)) {
+            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
                 strategy.deleteSnapshot(newSnapshot.getId());
             }
         }
@@ -444,8 +450,11 @@ public class SnapshotTest extends CloudStackTestNGBase {
         SnapshotVO snapshotVO = createSnapshotInDb(vol);
         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)) {
+            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
                 snapshot = strategy.takeSnapshot(snapshot);
                 result = true;
             }
@@ -474,8 +483,11 @@ public class SnapshotTest extends CloudStackTestNGBase {
         SnapshotVO snapshotVO = createSnapshotInDb(vol);
         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)) {
+            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
                 newSnapshot = strategy.takeSnapshot(snapshot);
             }
         }
@@ -487,7 +499,7 @@ public class SnapshotTest extends CloudStackTestNGBase {
 
         try {
             for (SnapshotStrategy strategy : this.snapshotStrategies) {
-                if (strategy.canHandle(snapshot)) {
+                if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
                     boolean res = strategy.deleteSnapshot(newSnapshot.getId());
                     Assert.assertTrue(res);
                 }

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

@@ -26,6 +26,7 @@ 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.SnapshotResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.storage.command.CreateObjectAnswer;
@@ -309,7 +310,7 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
     }
 
     @Override
-    public boolean canHandle(Snapshot snapshot) {
+    public StrategyPriority.Priority canHandle(Snapshot snapshot) {
-        return true;
+        return StrategyPriority.Priority.DEFAULT;
     }
 }

plugins/hypervisors/simulator/src/org/apache/cloudstack/storage/motion/SimulatorDataMotionStrategy.java

@@ -18,22 +18,28 @@
  */
 package org.apache.cloudstack.storage.motion;
 
-import com.cloud.agent.api.to.VirtualMachineTO;
+import java.util.Map;
-import com.cloud.host.Host;
+
-import org.apache.cloudstack.engine.subsystem.api.storage.*;
+import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy;
+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.StrategyPriority.Priority;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
 
-import java.util.Map;
+import com.cloud.agent.api.to.VirtualMachineTO;
+import com.cloud.host.Host;
 
 public class SimulatorDataMotionStrategy implements DataMotionStrategy {
     @Override
-    public boolean canHandle(DataObject srcData, DataObject destData) {
+    public Priority canHandle(DataObject srcData, DataObject destData) {
-        return true;
+        return Priority.HYPERVISOR;
     }
 
     @Override
-    public boolean canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+    public Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
-        return true;
+        return Priority.HYPERVISOR;
     }
 
     @Override

plugins/hypervisors/vmware/src/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java

@@ -21,12 +21,14 @@ package org.apache.cloudstack.storage.motion;
 
 import java.util.HashMap;
 import java.util.Map;
+
 import javax.inject.Inject;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy;
 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.StrategyPriority.Priority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
@@ -62,17 +64,17 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy {
     @Inject VMInstanceDao instanceDao;
 
     @Override
-    public boolean canHandle(DataObject srcData, DataObject destData) {
+    public Priority canHandle(DataObject srcData, DataObject destData) {
-        return false;
+        return Priority.CANT_HANDLE;
     }
 
     @Override
-    public boolean canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+    public Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
         if (srcHost.getHypervisorType() == HypervisorType.VMware && destHost.getHypervisorType() == HypervisorType.VMware) {
             s_logger.debug(this.getClass() + " can handle the request because the hosts have VMware hypervisor");
-            return true;
+            return Priority.HYPERVISOR;
         }
-        return false;
+        return Priority.CANT_HANDLE;
     }
 
     @Override

plugins/hypervisors/vmware/test/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java

@@ -16,13 +16,6 @@
 // under the License.
 package org.apache.cloudstack.storage.motion;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Matchers.anyLong;
-import static org.mockito.Matchers.isA;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
@@ -32,6 +25,7 @@ import javax.naming.ConfigurationException;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.async.AsyncCallFuture;
@@ -69,6 +63,14 @@ import com.cloud.utils.component.ComponentContext;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.dao.VMInstanceDao;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.isA;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration(loader = AnnotationConfigContextLoader.class)
 public class VmwareStorageMotionStrategyTest {
@@ -98,8 +100,8 @@ public class VmwareStorageMotionStrategyTest {
         when(srcHost.getHypervisorType()).thenReturn(HypervisorType.VMware);
         when(destHost.getHypervisorType()).thenReturn(HypervisorType.VMware);
         Map<VolumeInfo, DataStore> volumeMap = new HashMap<VolumeInfo, DataStore>();
-        boolean canHandle = strategy.canHandle(volumeMap, srcHost, destHost);
+        Priority canHandle = strategy.canHandle(volumeMap, srcHost, destHost);
-        assertTrue("The strategy is only supposed to handle vmware hosts", canHandle);
+        assertTrue("The strategy is only supposed to handle vmware hosts", canHandle == Priority.HYPERVISOR);
     }
 
     @Test
@@ -109,8 +111,8 @@ public class VmwareStorageMotionStrategyTest {
         when(srcHost.getHypervisorType()).thenReturn(HypervisorType.XenServer);
         when(destHost.getHypervisorType()).thenReturn(HypervisorType.XenServer);
         Map<VolumeInfo, DataStore> volumeMap = new HashMap<VolumeInfo, DataStore>();
-        boolean canHandle = strategy.canHandle(volumeMap, srcHost, destHost);
+        Priority canHandle = strategy.canHandle(volumeMap, srcHost, destHost);
-        assertFalse("The strategy is only supposed to handle vmware hosts", canHandle);
+        assertFalse("The strategy is only supposed to handle vmware hosts", canHandle == Priority.HYPERVISOR);
     }
 
     @Test
@@ -231,8 +233,8 @@ public class VmwareStorageMotionStrategyTest {
 
     @Configuration
     @ComponentScan(basePackageClasses = { VmwareStorageMotionStrategy.class },
-            includeFilters = {@Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)},
+    includeFilters = {@Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)},
-            useDefaultFilters = false)
+    useDefaultFilters = false)
     public static class TestConfiguration extends SpringUtils.CloudStackTestConfiguration {
 
         @Bean

plugins/hypervisors/xen/src/org/apache/cloudstack/storage/motion/XenServerStorageMotionStrategy.java

@@ -24,18 +24,18 @@ import java.util.Map;
 
 import javax.inject.Inject;
 
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
-
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy;
 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.StrategyPriority.Priority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.Answer;
@@ -71,18 +71,18 @@ public class XenServerStorageMotionStrategy implements DataMotionStrategy {
     @Inject VMInstanceDao instanceDao;
 
     @Override
-    public boolean canHandle(DataObject srcData, DataObject destData) {
+    public Priority canHandle(DataObject srcData, DataObject destData) {
-        return false;
+        return Priority.CANT_HANDLE;
     }
 
     @Override
-    public boolean canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+    public Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
-        boolean canHandle = false;
         if (srcHost.getHypervisorType() == HypervisorType.XenServer &&
                 destHost.getHypervisorType() == HypervisorType.XenServer) {
-            canHandle = true;
+            return Priority.HYPERVISOR;
         }
-        return canHandle;
+
+        return Priority.CANT_HANDLE;
     }
 
     @Override

server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java

@@ -40,6 +40,8 @@ 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.StrategyPriority.Priority;
 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.ZoneScope;
@@ -276,9 +278,11 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
             }
         }
 
+        StrategyPriority.sortStrategies(snapshotStrategies, snapshot);
+
         SnapshotStrategy snapshotStrategy = null;
         for (SnapshotStrategy strategy : snapshotStrategies) {
-            if (strategy.canHandle(snapshot)) {
+            if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
                 snapshotStrategy = strategy;
                 break;
             }
@@ -378,7 +382,7 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
 <<<<<<< 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);
+                 DownloadSnapshotFromSwiftCommand cmd = new DownloadSnapshotFromSwiftCommand(swift, secondaryStoragePoolUrl, dcId, accountId, volumeId, parent, backupUuid, _backupsnapshotwait);
 >>>>>>> master
                 Answer answer = _agentMgr.sendToSSVM(dcId, cmd);
                 if ((answer == null) || !answer.getResult()) {
@@ -513,10 +517,12 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
             throw new InvalidParameterValueException("unable to find a snapshot with id " + snapshotId);
         }
 
+        StrategyPriority.sortStrategies(snapshotStrategies, snapshotCheck);
+
         _accountMgr.checkAccess(caller, null, true, snapshotCheck);
         SnapshotStrategy snapshotStrategy = null;
         for (SnapshotStrategy strategy : snapshotStrategies) {
-            if (strategy.canHandle(snapshotCheck)) {
+            if (strategy.canHandle(snapshotCheck) != Priority.CANT_HANDLE) {
                 snapshotStrategy = strategy;
                 break;
             }
@@ -707,8 +713,11 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
             for (SnapshotVO snapshot : snapshots) {
                 SnapshotVO snap = _snapshotDao.findById(snapshot.getId());
                 SnapshotStrategy snapshotStrategy = null;
+
+                StrategyPriority.sortStrategies(snapshotStrategies, snapshot);
+
                 for (SnapshotStrategy strategy : snapshotStrategies) {
-                    if (strategy.canHandle(snap)) {
+                    if (strategy.canHandle(snap) != Priority.CANT_HANDLE) {
                         snapshotStrategy = strategy;
                         break;
                     }
@@ -1038,9 +1047,11 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
         SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId, volume.getDataStore());
         boolean processed = false;
 
+        StrategyPriority.sortStrategies(snapshotStrategies, snapshot);
+
         try {
             for (SnapshotStrategy strategy : snapshotStrategies) {
-                if (strategy.canHandle(snapshot)) {
+                if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) {
                     processed = true;
                     snapshot = strategy.takeSnapshot(snapshot);
                     break;