Return-Path: X-Original-To: apmail-cloudstack-commits-archive@www.apache.org Delivered-To: apmail-cloudstack-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 5D79910DB8 for ; Thu, 24 Oct 2013 18:35:26 +0000 (UTC) Received: (qmail 80092 invoked by uid 500); 24 Oct 2013 18:32:13 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 79275 invoked by uid 500); 24 Oct 2013 18:30:57 -0000 Mailing-List: contact commits-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list commits@cloudstack.apache.org Received: (qmail 79024 invoked by uid 99); 24 Oct 2013 18:30:41 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 24 Oct 2013 18:30:41 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id E510E81C07F; Thu, 24 Oct 2013 18:30:40 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: bfederle@apache.org To: commits@cloudstack.apache.org Date: Thu, 24 Oct 2013 18:31:01 -0000 Message-Id: In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [23/50] [abbrv] git commit: updated refs/heads/ui-restyle to 500c798 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 Authored: Wed Oct 23 12:40:41 2013 -0700 Committer: Darren Shepherd 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 volumeMap, Host srcHost, Host destHost); + StrategyPriority canHandle(Map volumeMap, Host srcHost, Host destHost); Void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback 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 getDataMotionStrategies(DataObject srcData, DataObject destData); + + DataMotionStrategy getDataMotionStrategy(DataObject srcData, DataObject destData); + + + Collection getDataMotionStrategies(Map volumeMap, Host srcHost, Host destHost); + + DataMotionStrategy getDataMotionStrategy(Map volumeMap, Host srcHost, Host destHost); + + + Collection 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 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 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 strategies, Map 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 strategies = new ArrayList(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 strategies = new ArrayList(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 strategies = new ArrayList(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 @@ - 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 volumeMap, Host srcHost, Host destHost) { - return Priority.CANT_HANDLE; + public StrategyPriority canHandle(Map 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 strategies; + @Inject + StorageStrategyFactory storageStrategyFactory; @Override public void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback 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 volumeMap, VirtualMachineTO vmTo, Host srcHost, Host destHost, AsyncCompletionCallback 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 volumeIds = new LinkedList(); 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 strategies) { - this.strategies = strategies; - } - - public List 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 @@ + + + + + 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 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 snapshotStrategies; + List dataMotionStrategies; + + @Override + public DataMotionStrategy getDataMotionStrategy(DataObject srcData, DataObject destData) { + return first(getDataMotionStrategies(srcData, destData)); + } + + @Override + public DataMotionStrategy getDataMotionStrategy(Map 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 getDataMotionStrategies(final DataObject srcData, final DataObject destData) { + return sort(dataMotionStrategies, new CanHandle() { + @Override + public StrategyPriority canHandle(DataMotionStrategy strategy) { + return strategy.canHandle(srcData, destData); + } + }); + } + + @Override + public Collection getDataMotionStrategies(final Map volumeMap, final Host srcHost, final Host destHost) { + return sort(dataMotionStrategies, new CanHandle() { + @Override + public StrategyPriority canHandle(DataMotionStrategy strategy) { + return strategy.canHandle(volumeMap, srcHost, destHost); + } + }); + } + + @Override + public Collection getSnapshotStrategies(final Snapshot snapshot, final SnapshotOperation op) { + return sort(snapshotStrategies, new CanHandle() { + @Override + public StrategyPriority canHandle(SnapshotStrategy strategy) { + return strategy.canHandle(snapshot, op); + } + }); + } + + private static Collection sort(Collection collection, final CanHandle canHandle) { + if (collection.size() == 0) + return null; + + TreeSet resultSet = new TreeSet(new Comparator() { + @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 first(Collection resultSet) { + return resultSet.size() == 0 ? null : resultSet.iterator().next(); + } + + private static interface CanHandle { + StrategyPriority canHandle(T strategy); + } + + public List getSnapshotStrategies() { + return snapshotStrategies; + } + + @Inject + public void setSnapshotStrategies(List snapshotStrategies) { + this.snapshotStrategies = snapshotStrategies; + } + + public List getDataMotionStrategies() { + return dataMotionStrategies; + } + + @Inject + public void setDataMotionStrategies(List 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 strategies = new ArrayList(5); + strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy)); + + StorageStrategyFactoryImpl factory = new StorageStrategyFactoryImpl(); + factory.setSnapshotStrategies(strategies); + Iterator 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 strategies = new ArrayList(5); + strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy)); + + StorageStrategyFactoryImpl factory = new StorageStrategyFactoryImpl(); + factory.setDataMotionStrategies(strategies); + Iterator 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 strategies = new ArrayList(5); + strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy)); + + StorageStrategyFactoryImpl factory = new StorageStrategyFactoryImpl(); + factory.setDataMotionStrategies(strategies); + Iterator 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 volumeMap, Host srcHost, Host destHost) { - return Priority.HYPERVISOR; + public StrategyPriority canHandle(Map 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 volumeMap, Host srcHost, Host destHost) { + public StrategyPriority canHandle(Map 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 volumeMap = new HashMap(); - 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 volumeMap = new HashMap(); - 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 volumeMap, Host srcHost, Host destHost) { + public StrategyPriority canHandle(Map 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 @@ - - - + 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 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 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 getSnapshotStrategies() { - return snapshotStrategies; - } - - @Inject - public void setSnapshotStrategies(List snapshotStrategies) { - this.snapshotStrategies = snapshotStrategies; - } }