cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bfede...@apache.org
Subject [23/50] [abbrv] git commit: updated refs/heads/ui-restyle to 500c798
Date Thu, 24 Oct 2013 18:31:01 GMT
Added StorageStrategyFactory to centralize and deduplicate some logic


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

Branch: refs/heads/ui-restyle
Commit: c9101966e0a76abb077556d946e140dc14088eb5
Parents: 81d0136
Author: Darren Shepherd <darren.s.shepherd@gmail.com>
Authored: Wed Oct 23 12:40:41 2013 -0700
Committer: Darren Shepherd <darren.s.shepherd@gmail.com>
Committed: Wed Oct 23 12:40:41 2013 -0700

----------------------------------------------------------------------
 .../api/storage/DataMotionStrategy.java         |   4 +-
 .../subsystem/api/storage/SnapshotStrategy.java |   2 +-
 .../api/storage/StorageStrategyFactory.java     |  45 ++++++
 .../subsystem/api/storage/StrategyPriority.java |  68 +--------
 .../api/storage/StrategyPriorityTest.java       | 152 -------------------
 ...g-engine-storage-datamotion-core-context.xml |   1 -
 .../motion/AncientDataMotionStrategy.java       |  10 +-
 .../storage/motion/DataMotionServiceImpl.java   |  21 +--
 .../core/spring-engine-storage-core-context.xml |   5 +
 .../storage/snapshot/SnapshotObject.java        |   5 +-
 .../snapshot/XenserverSnapshotStrategy.java     |   7 +-
 .../helper/StorageStrategyFactoryImpl.java      | 139 +++++++++++++++++
 .../api/storage/StrategyPriorityTest.java       | 122 +++++++++++++++
 .../motion/SimulatorDataMotionStrategy.java     |  10 +-
 .../motion/VmwareStorageMotionStrategy.java     |  12 +-
 .../motion/VmwareStorageMotionStrategyTest.java |  10 +-
 .../motion/XenServerStorageMotionStrategy.java  |  12 +-
 .../spring-server-core-managers-context.xml     |   4 +-
 .../storage/snapshot/SnapshotManagerImpl.java   |  21 +--
 19 files changed, 368 insertions(+), 282 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java
index 950f9e2..85fbf73 100644
--- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java
+++ b/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 {
-    StrategyPriority.Priority canHandle(DataObject srcData, DataObject destData);
+    StrategyPriority canHandle(DataObject srcData, DataObject destData);
 
-    StrategyPriority.Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost);
+    StrategyPriority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost);
 
     Void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback<CopyCommandResult> callback);
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java
index 3436d16..cf30f59 100644
--- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java
@@ -34,5 +34,5 @@ public interface SnapshotStrategy {
 
     boolean revertSnapshot(Long snapshotId);
 
-    StrategyPriority.Priority canHandle(Snapshot snapshot, SnapshotOperation op);
+    StrategyPriority canHandle(Snapshot snapshot, SnapshotOperation op);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/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
new file mode 100644
index 0000000..3a73cd0
--- /dev/null
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
@@ -0,0 +1,45 @@
+/*
+ * 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 java.util.Collection;
+import java.util.Map;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
+
+import com.cloud.host.Host;
+import com.cloud.storage.Snapshot;
+
+public interface StorageStrategyFactory {
+
+    Collection<DataMotionStrategy> getDataMotionStrategies(DataObject srcData, DataObject destData);
+
+    DataMotionStrategy getDataMotionStrategy(DataObject srcData, DataObject destData);
+
+
+    Collection<DataMotionStrategy> getDataMotionStrategies(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost);
+
+    DataMotionStrategy getDataMotionStrategy(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost);
+
+
+    Collection<SnapshotStrategy> getSnapshotStrategies(Snapshot snapshot, SnapshotOperation op);
+
+    SnapshotStrategy getSnapshotStrategy(Snapshot snapshot, SnapshotOperation op); 
+
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriority.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriority.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriority.java
index 05159dd..12f2a6a 100644
--- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriority.java
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriority.java
@@ -16,66 +16,10 @@
 // under the License.
 package org.apache.cloudstack.engine.subsystem.api.storage;
 
-import java.util.List;
-import java.util.Map;
-
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
-
-import com.cloud.host.Host;
-import com.cloud.storage.Snapshot;
-
-public class StrategyPriority {
-    public enum Priority {
-        CANT_HANDLE,
-        DEFAULT,
-        HYPERVISOR,
-        PLUGIN,
-        HIGHEST
-    }
-
-    public static SnapshotStrategy pickStrategy(List<SnapshotStrategy> strategies, Snapshot snapshot, SnapshotOperation op) {
-        Priority highestPriority = Priority.CANT_HANDLE;
-        SnapshotStrategy strategyToUse = null;
-
-        for (SnapshotStrategy strategy : strategies) {
-            Priority priority = strategy.canHandle(snapshot, op);
-            if (priority.ordinal() > highestPriority.ordinal()) {
-                highestPriority = priority;
-                strategyToUse = strategy;
-            }
-        }
-
-        return strategyToUse;
-    }
-
-    // TODO DRY this out by consolidating methods
-    public static DataMotionStrategy pickStrategy(List<DataMotionStrategy> strategies, DataObject srcData, DataObject destData) {
-        Priority highestPriority = Priority.CANT_HANDLE;
-        DataMotionStrategy strategyToUse = null;
-
-        for (DataMotionStrategy strategy : strategies) {
-            Priority priority = strategy.canHandle(srcData, destData);
-            if (priority.ordinal() > highestPriority.ordinal()) {
-                highestPriority = priority;
-                strategyToUse = strategy;
-            }
-        }
-
-        return strategyToUse;
-    }
-
-    public static DataMotionStrategy pickStrategy(List<DataMotionStrategy> strategies, Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
-        Priority highestPriority = Priority.CANT_HANDLE;
-        DataMotionStrategy strategyToUse = null;
-
-        for (DataMotionStrategy strategy : strategies) {
-            Priority priority = strategy.canHandle(volumeMap, srcHost, destHost);
-            if (priority.ordinal() > highestPriority.ordinal()) {
-                highestPriority = priority;
-                strategyToUse = strategy;
-            }
-        }
-
-        return strategyToUse;
-    }
+public enum StrategyPriority {
+    CANT_HANDLE,
+    DEFAULT,
+    HYPERVISOR,
+    PLUGIN,
+    HIGHEST
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/engine/api/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
----------------------------------------------------------------------
diff --git a/engine/api/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java b/engine/api/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
deleted file mode 100644
index e18e660..0000000
--- a/engine/api/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
+++ /dev/null
@@ -1,152 +0,0 @@
-// 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 java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
-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), any(SnapshotOperation.class));
-        doReturn(Priority.DEFAULT).when(defaultStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
-        doReturn(Priority.HYPERVISOR).when(hyperStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
-        doReturn(Priority.PLUGIN).when(pluginStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
-        doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
-
-        List<SnapshotStrategy> strategies = new ArrayList<SnapshotStrategy>(5);
-        SnapshotStrategy strategy = null;
-
-        strategies.add(cantHandleStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
-        assertEquals("A strategy was found when it shouldn't have been.", null, strategy);
-
-        strategies.add(defaultStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
-        assertEquals("Default strategy was not picked.", defaultStrategy, strategy);
-
-        strategies.add(hyperStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
-        assertEquals("Hypervisor strategy was not picked.", hyperStrategy, strategy);
-
-        strategies.add(pluginStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
-        assertEquals("Plugin strategy was not picked.", pluginStrategy, strategy);
-
-        strategies.add(highestStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
-        assertEquals("Highest strategy was not picked.", highestStrategy, strategy);
-    }
-
-    @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);
-        DataMotionStrategy strategy = null;
-
-        strategies.add(cantHandleStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
-        assertEquals("A strategy was found when it shouldn't have been.", null, strategy);
-
-        strategies.add(defaultStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
-        assertEquals("Default strategy was not picked.", defaultStrategy, strategy);
-
-        strategies.add(hyperStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
-        assertEquals("Hypervisor strategy was not picked.", hyperStrategy, strategy);
-
-        strategies.add(pluginStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
-        assertEquals("Plugin strategy was not picked.", pluginStrategy, strategy);
-
-        strategies.add(highestStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
-        assertEquals("Highest strategy was not picked.", highestStrategy, strategy);
-    }
-
-    @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);
-        DataMotionStrategy strategy = null;
-
-        strategies.add(cantHandleStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
-        assertEquals("A strategy was found when it shouldn't have been.", null, strategy);
-
-        strategies.add(defaultStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
-        assertEquals("Default strategy was not picked.", defaultStrategy, strategy);
-
-        strategies.add(hyperStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
-        assertEquals("Hypervisor strategy was not picked.", hyperStrategy, strategy);
-
-        strategies.add(pluginStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
-        assertEquals("Plugin strategy was not picked.", pluginStrategy, strategy);
-
-        strategies.add(highestStrategy);
-        strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
-        assertEquals("Highest strategy was not picked.", highestStrategy, strategy);
-    }
-}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/engine/storage/datamotion/resources/META-INF/cloudstack/core/spring-engine-storage-datamotion-core-context.xml
----------------------------------------------------------------------
diff --git a/engine/storage/datamotion/resources/META-INF/cloudstack/core/spring-engine-storage-datamotion-core-context.xml b/engine/storage/datamotion/resources/META-INF/cloudstack/core/spring-engine-storage-datamotion-core-context.xml
index 07c0fdf..3bde768 100644
--- a/engine/storage/datamotion/resources/META-INF/cloudstack/core/spring-engine-storage-datamotion-core-context.xml
+++ b/engine/storage/datamotion/resources/META-INF/cloudstack/core/spring-engine-storage-datamotion-core-context.xml
@@ -29,7 +29,6 @@
 
     <bean id="dataMotionServiceImpl"
         class="org.apache.cloudstack.storage.motion.DataMotionServiceImpl">
-        <property name="strategies" value="#{dataMotionStrategiesRegistry.registered}" />
     </bean>
 
 </beans>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/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..54e01f1 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
@@ -36,7 +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.StrategyPriority;
 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;
@@ -124,13 +124,13 @@ AncientDataMotionStrategy implements DataMotionStrategy {
     ManagementService _mgmtServer;
 
     @Override
-    public Priority canHandle(DataObject srcData, DataObject destData) {
-        return Priority.DEFAULT;
+    public StrategyPriority canHandle(DataObject srcData, DataObject destData) {
+        return StrategyPriority.DEFAULT;
     }
 
     @Override
-    public Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
-        return Priority.CANT_HANDLE;
+    public StrategyPriority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+        return StrategyPriority.CANT_HANDLE;
     }
 
     protected boolean needCacheStorage(DataObject srcData, DataObject destData) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java b/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java
index 47c988f..a1f05e1 100644
--- a/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java
+++ b/engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java
@@ -30,6 +30,8 @@ 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.StorageStrategyFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
 import org.springframework.stereotype.Component;
@@ -41,8 +43,8 @@ import com.cloud.utils.exception.CloudRuntimeException;
 
 @Component
 public class DataMotionServiceImpl implements DataMotionService {
-
-    List<DataMotionStrategy> strategies;
+    @Inject
+    StorageStrategyFactory storageStrategyFactory;
 
     @Override
     public void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback<CopyCommandResult> callback) {
@@ -58,8 +60,7 @@ public class DataMotionServiceImpl implements DataMotionService {
             return;
         }
 
-        // TODO DRY this out when the overloaded methods are DRYed out
-        DataMotionStrategy strategy = StrategyPriority.pickStrategy(strategies, srcData, destData);
+        DataMotionStrategy strategy = storageStrategyFactory.getDataMotionStrategy(srcData, destData);
         if (strategy == null) {
             throw new CloudRuntimeException("Can't find strategy to move data. "+
                     "Source: "+srcData.getType().name()+" '"+srcData.getUuid()+
@@ -73,8 +74,7 @@ public class DataMotionServiceImpl implements DataMotionService {
     public void copyAsync(Map<VolumeInfo, DataStore> volumeMap, VirtualMachineTO vmTo, Host srcHost, Host destHost,
             AsyncCompletionCallback<CopyCommandResult> callback) {
 
-        // TODO DRY this out when the overloaded methods are DRYed out
-        DataMotionStrategy strategy = StrategyPriority.pickStrategy(strategies, volumeMap, srcHost, destHost);
+        DataMotionStrategy strategy = storageStrategyFactory.getDataMotionStrategy(volumeMap, srcHost, destHost);
         if (strategy == null) {
             List<String> volumeIds = new LinkedList<String>();
             for (final VolumeInfo volumeInfo : volumeMap.keySet()) {
@@ -88,13 +88,4 @@ public class DataMotionServiceImpl implements DataMotionService {
 
         strategy.copyAsync(volumeMap, vmTo, srcHost, destHost, callback);
     }
-
-    @Inject
-    public void setStrategies(List<DataMotionStrategy> strategies) {
-        this.strategies = strategies;
-    }
-
-    public List<DataMotionStrategy> getStrategies() {
-        return strategies;
-    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/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 29f4fcd..8a78fdd 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
@@ -59,5 +59,10 @@
             <list />
         </property>
     </bean>
+    
+    <bean id="storageStrategyFactoryImpl" class="org.apache.cloudstack.storage.helper.StorageStrategyFactoryImpl" >
+        <property name="dataMotionStrategies" value="#{dataMotionStrategiesRegistry.registered}" />
+        <property name="snapshotStrategies" value="#{snapshotStrategiesRegistry.registered}" />
+    </bean>
 
 </beans>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java
index 147f1d4..daf6477 100644
--- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java
@@ -31,6 +31,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
 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.StrategyPriority;
+import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.storage.command.CopyCmdAnswer;
@@ -76,7 +77,7 @@ public class SnapshotObject implements SnapshotInfo {
     @Inject
     SnapshotDataStoreDao snapshotStoreDao;
     @Inject
-    List<SnapshotStrategy> snapshotStrategies;
+    StorageStrategyFactory storageStrategyFactory;
 
     public SnapshotObject() {
 
@@ -129,7 +130,7 @@ public class SnapshotObject implements SnapshotInfo {
 
     @Override
     public boolean isRevertable() {
-        SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.REVERT);
+        SnapshotStrategy snapshotStrategy = storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.REVERT);
         if (snapshotStrategy != null) {
             return true;
         }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/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 b3a64b6..27dbd83 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
@@ -27,7 +27,6 @@ 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.StrategyPriority.Priority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.storage.command.CreateObjectAnswer;
@@ -311,11 +310,11 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
     }
 
     @Override
-    public StrategyPriority.Priority canHandle(Snapshot snapshot, SnapshotOperation op) {
+    public StrategyPriority canHandle(Snapshot snapshot, SnapshotOperation op) {
         if (op == SnapshotOperation.REVERT) {
-            return Priority.CANT_HANDLE;
+            return StrategyPriority.CANT_HANDLE;
         }
 
-        return Priority.DEFAULT;
+        return StrategyPriority.DEFAULT;
     }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/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
new file mode 100644
index 0000000..0bbd28b
--- /dev/null
+++ b/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
@@ -0,0 +1,139 @@
+/*
+ * 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.helper;
+
+import java.util.Collection;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Map;
+import java.util.TreeSet;
+
+import javax.inject.Inject;
+
+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.SnapshotStrategy;
+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.VolumeInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
+
+import com.cloud.host.Host;
+import com.cloud.storage.Snapshot;
+
+public class StorageStrategyFactoryImpl implements StorageStrategyFactory {
+
+    List<SnapshotStrategy> snapshotStrategies;
+    List<DataMotionStrategy> dataMotionStrategies;
+
+    @Override
+    public DataMotionStrategy getDataMotionStrategy(DataObject srcData, DataObject destData) {
+        return first(getDataMotionStrategies(srcData, destData));
+    }
+
+    @Override
+    public DataMotionStrategy getDataMotionStrategy(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+        return first(getDataMotionStrategies(volumeMap, srcHost, destHost));
+    }
+
+    @Override
+    public SnapshotStrategy getSnapshotStrategy(Snapshot snapshot, SnapshotOperation op) {
+        return first(getSnapshotStrategies(snapshot, op));
+    }
+
+    @Override
+    public Collection<DataMotionStrategy> getDataMotionStrategies(final DataObject srcData, final DataObject destData) {
+        return sort(dataMotionStrategies, new CanHandle<DataMotionStrategy>() {
+            @Override
+            public StrategyPriority canHandle(DataMotionStrategy strategy) {
+                return strategy.canHandle(srcData, destData);
+            }
+        });
+    }
+
+    @Override
+    public Collection<DataMotionStrategy> getDataMotionStrategies(final Map<VolumeInfo, DataStore> volumeMap, final Host srcHost, final Host destHost) {
+        return sort(dataMotionStrategies, new CanHandle<DataMotionStrategy>() {
+            @Override
+            public StrategyPriority canHandle(DataMotionStrategy strategy) {
+                return strategy.canHandle(volumeMap, srcHost, destHost);
+            }
+        });
+    }
+
+    @Override
+    public Collection<SnapshotStrategy> getSnapshotStrategies(final Snapshot snapshot, final SnapshotOperation op) {
+        return sort(snapshotStrategies, new CanHandle<SnapshotStrategy>() {
+            @Override
+            public StrategyPriority canHandle(SnapshotStrategy strategy) {
+                return strategy.canHandle(snapshot, op);
+            }
+        });
+    }
+
+    private static <T> Collection<T> sort(Collection<T> collection, final CanHandle<T> canHandle) {
+        if (collection.size() == 0)
+            return null;
+
+        TreeSet<T> resultSet = new TreeSet<T>(new Comparator<T>() {
+            @Override
+            public int compare(T o1, T o2) {
+                int i1 = canHandle.canHandle(o1).ordinal();
+                int i2 = canHandle.canHandle(o2).ordinal();
+                return new Integer(i2).compareTo(new Integer(i1));
+            }
+        });
+
+        for ( T test : collection ) {
+            if ( canHandle.canHandle(test) != StrategyPriority.CANT_HANDLE ) {
+                resultSet.add(test);
+            }
+        }
+
+        return resultSet;
+    }
+    
+    private static <T> T first(Collection<T> resultSet) {
+        return resultSet.size() == 0 ? null : resultSet.iterator().next();
+    }
+    
+    private static interface CanHandle<T> {
+        StrategyPriority canHandle(T strategy);
+    }
+
+    public List<SnapshotStrategy> getSnapshotStrategies() {
+        return snapshotStrategies;
+    }
+
+    @Inject
+    public void setSnapshotStrategies(List<SnapshotStrategy> snapshotStrategies) {
+        this.snapshotStrategies = snapshotStrategies;
+    }
+
+    public List<DataMotionStrategy> getDataMotionStrategies() {
+        return dataMotionStrategies;
+    }
+
+    @Inject
+    public void setDataMotionStrategies(List<DataMotionStrategy> dataMotionStrategies) {
+        this.dataMotionStrategies = dataMotionStrategies;
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
----------------------------------------------------------------------
diff --git a/engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java b/engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
new file mode 100644
index 0000000..8cfa955
--- /dev/null
+++ b/engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
@@ -0,0 +1,122 @@
+// 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 static org.junit.Assert.*;
+import static org.mockito.Matchers.*;
+import static org.mockito.Mockito.*;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
+import org.apache.cloudstack.storage.helper.StorageStrategyFactoryImpl;
+import org.junit.Test;
+
+import com.cloud.host.Host;
+import com.cloud.storage.Snapshot;
+
+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(StrategyPriority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
+        doReturn(StrategyPriority.DEFAULT).when(defaultStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
+        doReturn(StrategyPriority.HYPERVISOR).when(hyperStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
+        doReturn(StrategyPriority.PLUGIN).when(pluginStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
+        doReturn(StrategyPriority.HIGHEST).when(highestStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
+
+        List<SnapshotStrategy> strategies = new ArrayList<SnapshotStrategy>(5);
+        strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy));
+
+        StorageStrategyFactoryImpl factory = new StorageStrategyFactoryImpl();
+        factory.setSnapshotStrategies(strategies);
+        Iterator<SnapshotStrategy> iter = factory.getSnapshotStrategies(mock(Snapshot.class), SnapshotOperation.TAKE).iterator();
+
+        assertEquals("Highest was not 1st.", highestStrategy, iter.next());
+        assertEquals("Plugin was not 2nd.", pluginStrategy, iter.next());
+        assertEquals("Hypervisor was not 3rd.", hyperStrategy, iter.next());
+        assertEquals("Default was not 4th.", defaultStrategy, iter.next());
+        assertTrue("Can't Handle was not 5th.", !iter.hasNext());
+    }
+
+    @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(StrategyPriority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(DataObject.class), any(DataObject.class));
+        doReturn(StrategyPriority.DEFAULT).when(defaultStrategy).canHandle(any(DataObject.class), any(DataObject.class));
+        doReturn(StrategyPriority.HYPERVISOR).when(hyperStrategy).canHandle(any(DataObject.class), any(DataObject.class));
+        doReturn(StrategyPriority.PLUGIN).when(pluginStrategy).canHandle(any(DataObject.class), any(DataObject.class));
+        doReturn(StrategyPriority.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));
+
+        StorageStrategyFactoryImpl factory = new StorageStrategyFactoryImpl();
+        factory.setDataMotionStrategies(strategies);
+        Iterator<DataMotionStrategy> iter = factory.getDataMotionStrategies(mock(DataObject.class), mock(DataObject.class)).iterator();
+
+        assertEquals("Highest was not 1st.", highestStrategy, iter.next());
+        assertEquals("Plugin was not 2nd.", pluginStrategy, iter.next());
+        assertEquals("Hypervisor was not 3rd.", hyperStrategy, iter.next());
+        assertEquals("Default was not 4th.", defaultStrategy, iter.next());
+        assertTrue("Can't Handle was not 5th.", !iter.hasNext());
+    }
+
+    @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(StrategyPriority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
+        doReturn(StrategyPriority.DEFAULT).when(defaultStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
+        doReturn(StrategyPriority.HYPERVISOR).when(hyperStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
+        doReturn(StrategyPriority.PLUGIN).when(pluginStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
+        doReturn(StrategyPriority.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));
+
+        StorageStrategyFactoryImpl factory = new StorageStrategyFactoryImpl();
+        factory.setDataMotionStrategies(strategies);
+        Iterator<DataMotionStrategy> iter = factory.getDataMotionStrategies(mock(Map.class), mock(Host.class), mock(Host.class)).iterator();
+
+        assertEquals("Highest was not 1st.", highestStrategy, iter.next());
+        assertEquals("Plugin was not 2nd.", pluginStrategy, iter.next());
+        assertEquals("Hypervisor was not 3rd.", hyperStrategy, iter.next());
+        assertEquals("Default was not 4th.", defaultStrategy, iter.next());
+        assertTrue("Can't Handle was not 5th.", !iter.hasNext());
+    }
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/plugins/hypervisors/simulator/src/org/apache/cloudstack/storage/motion/SimulatorDataMotionStrategy.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/simulator/src/org/apache/cloudstack/storage/motion/SimulatorDataMotionStrategy.java b/plugins/hypervisors/simulator/src/org/apache/cloudstack/storage/motion/SimulatorDataMotionStrategy.java
index 74868c4..3eb2cf6 100644
--- a/plugins/hypervisors/simulator/src/org/apache/cloudstack/storage/motion/SimulatorDataMotionStrategy.java
+++ b/plugins/hypervisors/simulator/src/org/apache/cloudstack/storage/motion/SimulatorDataMotionStrategy.java
@@ -24,7 +24,7 @@ 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.StrategyPriority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
 import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
 
@@ -33,13 +33,13 @@ import com.cloud.host.Host;
 
 public class SimulatorDataMotionStrategy implements DataMotionStrategy {
     @Override
-    public Priority canHandle(DataObject srcData, DataObject destData) {
-        return Priority.HYPERVISOR;
+    public StrategyPriority canHandle(DataObject srcData, DataObject destData) {
+        return StrategyPriority.HYPERVISOR;
     }
 
     @Override
-    public Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
-        return Priority.HYPERVISOR;
+    public StrategyPriority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+        return StrategyPriority.HYPERVISOR;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/plugins/hypervisors/vmware/src/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/src/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
index f5f23a2..5c9b3af 100644
--- a/plugins/hypervisors/vmware/src/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
+++ b/plugins/hypervisors/vmware/src/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
@@ -28,7 +28,7 @@ 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.StrategyPriority;
 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;
@@ -64,17 +64,17 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy {
     @Inject VMInstanceDao instanceDao;
 
     @Override
-    public Priority canHandle(DataObject srcData, DataObject destData) {
-        return Priority.CANT_HANDLE;
+    public StrategyPriority canHandle(DataObject srcData, DataObject destData) {
+        return StrategyPriority.CANT_HANDLE;
     }
 
     @Override
-    public Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+    public StrategyPriority 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 Priority.HYPERVISOR;
+            return StrategyPriority.HYPERVISOR;
         }
-        return Priority.CANT_HANDLE;
+        return StrategyPriority.CANT_HANDLE;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/plugins/hypervisors/vmware/test/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/test/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java b/plugins/hypervisors/vmware/test/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java
index c19480c..1234728 100644
--- a/plugins/hypervisors/vmware/test/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java
+++ b/plugins/hypervisors/vmware/test/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategyTest.java
@@ -25,7 +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.StrategyPriority;
 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;
@@ -100,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>();
-        Priority canHandle = strategy.canHandle(volumeMap, srcHost, destHost);
-        assertTrue("The strategy is only supposed to handle vmware hosts", canHandle == Priority.HYPERVISOR);
+        StrategyPriority canHandle = strategy.canHandle(volumeMap, srcHost, destHost);
+        assertTrue("The strategy is only supposed to handle vmware hosts", canHandle == StrategyPriority.HYPERVISOR);
     }
 
     @Test
@@ -111,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>();
-        Priority canHandle = strategy.canHandle(volumeMap, srcHost, destHost);
-        assertFalse("The strategy is only supposed to handle vmware hosts", canHandle == Priority.HYPERVISOR);
+        StrategyPriority canHandle = strategy.canHandle(volumeMap, srcHost, destHost);
+        assertFalse("The strategy is only supposed to handle vmware hosts", canHandle == StrategyPriority.HYPERVISOR);
     }
 
     @Test

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/plugins/hypervisors/xen/src/org/apache/cloudstack/storage/motion/XenServerStorageMotionStrategy.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/xen/src/org/apache/cloudstack/storage/motion/XenServerStorageMotionStrategy.java b/plugins/hypervisors/xen/src/org/apache/cloudstack/storage/motion/XenServerStorageMotionStrategy.java
index 8578a9a..3dc7dd8 100644
--- a/plugins/hypervisors/xen/src/org/apache/cloudstack/storage/motion/XenServerStorageMotionStrategy.java
+++ b/plugins/hypervisors/xen/src/org/apache/cloudstack/storage/motion/XenServerStorageMotionStrategy.java
@@ -28,7 +28,7 @@ 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.StrategyPriority;
 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;
@@ -71,18 +71,18 @@ public class XenServerStorageMotionStrategy implements DataMotionStrategy {
     @Inject VMInstanceDao instanceDao;
 
     @Override
-    public Priority canHandle(DataObject srcData, DataObject destData) {
-        return Priority.CANT_HANDLE;
+    public StrategyPriority canHandle(DataObject srcData, DataObject destData) {
+        return StrategyPriority.CANT_HANDLE;
     }
 
     @Override
-    public Priority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
+    public StrategyPriority canHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
         if (srcHost.getHypervisorType() == HypervisorType.XenServer &&
                 destHost.getHypervisorType() == HypervisorType.XenServer) {
-            return Priority.HYPERVISOR;
+            return StrategyPriority.HYPERVISOR;
         }
 
-        return Priority.CANT_HANDLE;
+        return StrategyPriority.CANT_HANDLE;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
----------------------------------------------------------------------
diff --git a/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml b/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
index 7400759..11b69d2 100644
--- a/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
+++ b/server/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml
@@ -142,9 +142,7 @@
 
     <bean id="site2SiteVpnManagerImpl" class="com.cloud.network.vpn.Site2SiteVpnManagerImpl" />
 
-    <bean id="snapshotManagerImpl" class="com.cloud.storage.snapshot.SnapshotManagerImpl" >
-        <property name="snapshotStrategies" value="#{snapshotStrategiesRegistry.registered}" />
-    </bean>
+    <bean id="snapshotManagerImpl" class="com.cloud.storage.snapshot.SnapshotManagerImpl" />
 
     <bean id="snapshotSchedulerImpl" class="com.cloud.storage.snapshot.SnapshotSchedulerImpl" />
     <bean id="storageNetworkManagerImpl" class="com.cloud.network.StorageNetworkManagerImpl" />

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c9101966/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 a7a5e03..1e17de1 100755
--- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -42,6 +42,9 @@ 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.SnapshotStrategy.SnapshotOperation;
 import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
+import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
 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;
@@ -194,7 +197,7 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
     @Inject EndPointSelector _epSelector;
     @Inject
     private ResourceManager _resourceMgr;
-    protected List<SnapshotStrategy> snapshotStrategies;
+    @Inject StorageStrategyFactory _storageStrategyFactory;
 
 
     private int _totalRetries;
@@ -277,7 +280,7 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
             }
         }
 
-        SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.REVERT);
+        SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.REVERT);
 
         if (snapshotStrategy == null) {
             s_logger.error("Unable to find snaphot strategy to handle snapshot with id '"+snapshotId+"'");
@@ -506,7 +509,7 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
         }
 
         _accountMgr.checkAccess(caller, null, true, snapshotCheck);
-        SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshotCheck, SnapshotOperation.DELETE);
+        SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshotCheck, SnapshotOperation.DELETE);
         if (snapshotStrategy == null) {
             s_logger.error("Unable to find snaphot strategy to handle snapshot with id '"+snapshotId+"'");
             return false;
@@ -696,7 +699,7 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
             // Either way delete the snapshots for this volume.
             List<SnapshotVO> snapshots = listSnapsforVolume(volumeId);
             for (SnapshotVO snapshot : snapshots) {
-                SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.DELETE);
+                SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.DELETE);
                 if (snapshotStrategy == null) {
                     s_logger.error("Unable to find snaphot strategy to handle snapshot with id '"+snapshot.getId()+"'");
                     continue;
@@ -1027,7 +1030,7 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
         SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId, volume.getDataStore());
 
         try {
-            SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.TAKE);
+            SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.TAKE);
 
             if (snapshotStrategy == null) {
                 throw new CloudRuntimeException("Can't find snapshot strategy to deal with snapshot:" + snapshotId);
@@ -1210,12 +1213,4 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager,
         return snapshot;
     }
 
-    public List<SnapshotStrategy> getSnapshotStrategies() {
-        return snapshotStrategies;
-    }
-
-    @Inject
-    public void setSnapshotStrategies(List<SnapshotStrategy> snapshotStrategies) {
-        this.snapshotStrategies = snapshotStrategies;
-    }
 }


Mime
View raw message