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 D0E6D100C3 for ; Tue, 15 Oct 2013 20:20:30 +0000 (UTC) Received: (qmail 19563 invoked by uid 500); 15 Oct 2013 20:20:02 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 19124 invoked by uid 500); 15 Oct 2013 20:19:58 -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 19045 invoked by uid 99); 15 Oct 2013 20:19:57 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 15 Oct 2013 20:19:57 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id A418A8B5187; Tue, 15 Oct 2013 20:19:56 +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: Tue, 15 Oct 2013 20:20:05 -0000 Message-Id: <26329904b28b4b45bece1344ba2b1282@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [10/25] git commit: updated refs/heads/ui-restyle to 3fdb61f Added categorized sorting to SnapshotStrategy and DataMotionStrategy Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/aad1cda7 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/aad1cda7 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/aad1cda7 Branch: refs/heads/ui-restyle Commit: aad1cda7e08fc07eb2c1581e69db96d7cd42366e Parents: e514da2 Author: Chris Suich Authored: Fri Oct 11 15:20:22 2013 -0400 Committer: Edison Su Committed: Mon Oct 14 15:20:42 2013 -0700 ---------------------------------------------------------------------- .../api/storage/DataMotionStrategy.java | 4 +- .../subsystem/api/storage/SnapshotStrategy.java | 2 +- .../subsystem/api/storage/StrategyPriority.java | 83 +++++++++++++++ .../api/storage/StrategyPriorityTest.java | 100 +++++++++++++++++++ .../motion/AncientDataMotionStrategy.java | 10 +- .../storage/motion/DataMotionServiceImpl.java | 25 +++-- .../storage/test/MockStorageMotionStrategy.java | 23 ++--- .../cloudstack/storage/test/SnapshotTest.java | 26 +++-- .../snapshot/XenserverSnapshotStrategy.java | 5 +- .../motion/SimulatorDataMotionStrategy.java | 22 ++-- .../motion/VmwareStorageMotionStrategy.java | 12 ++- .../motion/VmwareStorageMotionStrategyTest.java | 28 +++--- .../motion/XenServerStorageMotionStrategy.java | 18 ++-- .../storage/snapshot/SnapshotManagerImpl.java | 21 +++- 14 files changed, 302 insertions(+), 77 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 6deb6c1..950f9e2 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 { - boolean canHandle(DataObject srcData, DataObject destData); + StrategyPriority.Priority canHandle(DataObject srcData, DataObject destData); - boolean canHandle(Map volumeMap, Host srcHost, Host destHost); + StrategyPriority.Priority 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/aad1cda7/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 47e595b..e4cecb6 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 @@ -27,5 +27,5 @@ public interface SnapshotStrategy { boolean revertSnapshot(Long snapshotId); - boolean canHandle(Snapshot snapshot); + StrategyPriority.Priority canHandle(Snapshot snapshot); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 new file mode 100644 index 0000000..8bd04f5 --- /dev/null +++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriority.java @@ -0,0 +1,83 @@ +package org.apache.cloudstack.engine.subsystem.api.storage; + +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Map; + +import com.cloud.host.Host; +import com.cloud.storage.Snapshot; + +public class StrategyPriority { + public enum Priority { + CANT_HANDLE, + DEFAULT, + HYPERVISOR, + PLUGIN, + HIGHEST + } + + public static void sortStrategies(List strategies, Snapshot snapshot) { + Collections.sort(strategies, new SnapshotStrategyComparator(snapshot)); + } + + public static void sortStrategies(List strategies, DataObject srcData, DataObject destData) { + Collections.sort(strategies, new DataMotionStrategyComparator(srcData, destData)); + } + + public static void sortStrategies(List strategies, Map volumeMap, Host srcHost, Host destHost) { + Collections.sort(strategies, new DataMotionStrategyHostComparator(volumeMap, srcHost, destHost)); + } + + static class SnapshotStrategyComparator implements Comparator { + + Snapshot snapshot; + + public SnapshotStrategyComparator(Snapshot snapshot) { + this.snapshot = snapshot; + } + + @Override + public int compare(SnapshotStrategy o1, SnapshotStrategy o2) { + int i1 = o1.canHandle(snapshot).ordinal(); + int i2 = o2.canHandle(snapshot).ordinal(); + return new Integer(i2).compareTo(new Integer(i1)); + } + } + + static class DataMotionStrategyComparator implements Comparator { + + DataObject srcData, destData; + + public DataMotionStrategyComparator(DataObject srcData, DataObject destData) { + this.srcData = srcData; + this.destData = destData; + } + + @Override + public int compare(DataMotionStrategy o1, DataMotionStrategy o2) { + int i1 = o1.canHandle(srcData, destData).ordinal(); + int i2 = o2.canHandle(srcData, destData).ordinal(); + return new Integer(i2).compareTo(new Integer(i1)); + } + } + + static class DataMotionStrategyHostComparator implements Comparator { + + Host srcHost, destHost; + Map volumeMap; + + public DataMotionStrategyHostComparator(Map volumeMap, Host srcHost, Host destHost) { + this.volumeMap = volumeMap; + this.srcHost = srcHost; + this.destHost = destHost; + } + + @Override + public int compare(DataMotionStrategy o1, DataMotionStrategy o2) { + int i1 = o1.canHandle(volumeMap, srcHost, destHost).ordinal(); + int i2 = o2.canHandle(volumeMap, srcHost, destHost).ordinal(); + return new Integer(i2).compareTo(new Integer(i1)); + } + } +} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 new file mode 100644 index 0000000..329b99f --- /dev/null +++ b/engine/api/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java @@ -0,0 +1,100 @@ +package org.apache.cloudstack.engine.subsystem.api.storage; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; +import org.junit.Test; + +import com.cloud.host.Host; +import com.cloud.storage.Snapshot; + +import static org.junit.Assert.assertEquals; + +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +public class StrategyPriorityTest { + + @Test + public void testSortSnapshotStrategies() { + SnapshotStrategy cantHandleStrategy = mock(SnapshotStrategy.class); + SnapshotStrategy defaultStrategy = mock(SnapshotStrategy.class); + SnapshotStrategy hyperStrategy = mock(SnapshotStrategy.class); + SnapshotStrategy pluginStrategy = mock(SnapshotStrategy.class); + SnapshotStrategy highestStrategy = mock(SnapshotStrategy.class); + + doReturn(Priority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(Snapshot.class)); + doReturn(Priority.DEFAULT).when(defaultStrategy).canHandle(any(Snapshot.class)); + doReturn(Priority.HYPERVISOR).when(hyperStrategy).canHandle(any(Snapshot.class)); + doReturn(Priority.PLUGIN).when(pluginStrategy).canHandle(any(Snapshot.class)); + doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(Snapshot.class)); + + List strategies = new ArrayList(5); + strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy)); + + StrategyPriority.sortStrategies(strategies, mock(Snapshot.class)); + + assertEquals("Highest was not 1st.", highestStrategy, strategies.get(0)); + assertEquals("Plugin was not 2nd.", pluginStrategy, strategies.get(1)); + assertEquals("Hypervisor was not 3rd.", hyperStrategy, strategies.get(2)); + assertEquals("Default was not 4th.", defaultStrategy, strategies.get(3)); + assertEquals("Can't Handle was not 5th.", cantHandleStrategy, strategies.get(4)); + } + + @Test + public void testSortDataMotionStrategies() { + DataMotionStrategy cantHandleStrategy = mock(DataMotionStrategy.class); + DataMotionStrategy defaultStrategy = mock(DataMotionStrategy.class); + DataMotionStrategy hyperStrategy = mock(DataMotionStrategy.class); + DataMotionStrategy pluginStrategy = mock(DataMotionStrategy.class); + DataMotionStrategy highestStrategy = mock(DataMotionStrategy.class); + + doReturn(Priority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(DataObject.class), any(DataObject.class)); + doReturn(Priority.DEFAULT).when(defaultStrategy).canHandle(any(DataObject.class), any(DataObject.class)); + doReturn(Priority.HYPERVISOR).when(hyperStrategy).canHandle(any(DataObject.class), any(DataObject.class)); + doReturn(Priority.PLUGIN).when(pluginStrategy).canHandle(any(DataObject.class), any(DataObject.class)); + doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(DataObject.class), any(DataObject.class)); + + List strategies = new ArrayList(5); + strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy)); + + StrategyPriority.sortStrategies(strategies, mock(DataObject.class), mock(DataObject.class)); + + assertEquals("Highest was not 1st.", highestStrategy, strategies.get(0)); + assertEquals("Plugin was not 2nd.", pluginStrategy, strategies.get(1)); + assertEquals("Hypervisor was not 3rd.", hyperStrategy, strategies.get(2)); + assertEquals("Default was not 4th.", defaultStrategy, strategies.get(3)); + assertEquals("Can't Handle was not 5th.", cantHandleStrategy, strategies.get(4)); + } + + @Test + @SuppressWarnings("unchecked") + public void testSortDataMotionStrategies2() { + DataMotionStrategy cantHandleStrategy = mock(DataMotionStrategy.class); + DataMotionStrategy defaultStrategy = mock(DataMotionStrategy.class); + DataMotionStrategy hyperStrategy = mock(DataMotionStrategy.class); + DataMotionStrategy pluginStrategy = mock(DataMotionStrategy.class); + DataMotionStrategy highestStrategy = mock(DataMotionStrategy.class); + + doReturn(Priority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class)); + doReturn(Priority.DEFAULT).when(defaultStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class)); + doReturn(Priority.HYPERVISOR).when(hyperStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class)); + doReturn(Priority.PLUGIN).when(pluginStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class)); + doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class)); + + List strategies = new ArrayList(5); + strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy)); + + StrategyPriority.sortStrategies(strategies, mock(Map.class), mock(Host.class), mock(Host.class)); + + assertEquals("Highest was not 1st.", highestStrategy, strategies.get(0)); + assertEquals("Plugin was not 2nd.", pluginStrategy, strategies.get(1)); + assertEquals("Hypervisor was not 3rd.", hyperStrategy, strategies.get(2)); + assertEquals("Default was not 4th.", defaultStrategy, strategies.get(3)); + assertEquals("Can't Handle was not 5th.", cantHandleStrategy, strategies.get(4)); + } +} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 fb6962a..5f5f01e 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,6 +36,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreState import org.apache.cloudstack.engine.subsystem.api.storage.Scope; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; @@ -123,14 +124,13 @@ AncientDataMotionStrategy implements DataMotionStrategy { ManagementService _mgmtServer; @Override - public boolean canHandle(DataObject srcData, DataObject destData) { - // TODO Auto-generated method stub - return true; + public Priority canHandle(DataObject srcData, DataObject destData) { + return Priority.DEFAULT; } @Override - public boolean canHandle(Map volumeMap, Host srcHost, Host destHost) { - return false; + public Priority canHandle(Map volumeMap, Host srcHost, Host destHost) { + return Priority.CANT_HANDLE; } protected boolean needCacheStorage(DataObject srcData, DataObject destData) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 9f0f531..2d31320 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 @@ -18,21 +18,25 @@ */ package org.apache.cloudstack.storage.motion; -import com.cloud.agent.api.to.VirtualMachineTO; -import com.cloud.host.Host; -import com.cloud.utils.exception.CloudRuntimeException; +import java.util.List; +import java.util.Map; + +import javax.inject.Inject; + import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionService; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.springframework.stereotype.Component; -import javax.inject.Inject; -import java.util.List; -import java.util.Map; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.host.Host; +import com.cloud.utils.exception.CloudRuntimeException; @Component public class DataMotionServiceImpl implements DataMotionService { @@ -53,8 +57,10 @@ public class DataMotionServiceImpl implements DataMotionService { return; } + StrategyPriority.sortStrategies(strategies, srcData, destData); + for (DataMotionStrategy strategy : strategies) { - if (strategy.canHandle(srcData, destData)) { + if (strategy.canHandle(srcData, destData) != Priority.CANT_HANDLE) { strategy.copyAsync(srcData, destData, callback); return; } @@ -65,8 +71,11 @@ public class DataMotionServiceImpl implements DataMotionService { @Override public void copyAsync(Map volumeMap, VirtualMachineTO vmTo, Host srcHost, Host destHost, AsyncCompletionCallback callback) { + + StrategyPriority.sortStrategies(strategies, volumeMap, srcHost, destHost); + for (DataMotionStrategy strategy : strategies) { - if (strategy.canHandle(volumeMap, srcHost, destHost)) { + if (strategy.canHandle(volumeMap, srcHost, destHost) != Priority.CANT_HANDLE) { strategy.copyAsync(volumeMap, vmTo, srcHost, destHost, callback); return; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/MockStorageMotionStrategy.java ---------------------------------------------------------------------- diff --git a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/MockStorageMotionStrategy.java b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/MockStorageMotionStrategy.java index 6c0bd55..79ed61b 100644 --- a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/MockStorageMotionStrategy.java +++ b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/MockStorageMotionStrategy.java @@ -21,31 +21,30 @@ package org.apache.cloudstack.storage.test; import java.util.Map; import java.util.UUID; -import com.cloud.agent.api.to.DataObjectType; -import com.cloud.agent.api.to.DataTO; -import com.cloud.storage.Storage; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; - -import com.cloud.agent.api.to.VirtualMachineTO; -import com.cloud.host.Host; import org.apache.cloudstack.storage.command.CopyCmdAnswer; -import org.apache.cloudstack.storage.snapshot.SnapshotObject; import org.apache.cloudstack.storage.to.SnapshotObjectTO; import org.apache.cloudstack.storage.to.TemplateObjectTO; +import com.cloud.agent.api.to.DataObjectType; +import com.cloud.agent.api.to.DataTO; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.host.Host; +import com.cloud.storage.Storage; + public class MockStorageMotionStrategy implements DataMotionStrategy { boolean success = true; @Override - public boolean canHandle(DataObject srcData, DataObject destData) { - // TODO Auto-generated method stub - return true; + public Priority canHandle(DataObject srcData, DataObject destData) { + return Priority.HIGHEST; } public void makeBackupSnapshotSucceed(boolean success) { @@ -53,8 +52,8 @@ public class MockStorageMotionStrategy implements DataMotionStrategy { } @Override - public boolean canHandle(Map volumeMap, Host srcHost, Host destHost) { - return true; + public Priority canHandle(Map volumeMap, Host srcHost, Host destHost) { + return Priority.HIGHEST; } @Override http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java ---------------------------------------------------------------------- diff --git a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java index f1eed3a..81f77d6 100644 --- a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java +++ b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java @@ -400,8 +400,11 @@ public class SnapshotTest extends CloudStackTestNGBase { SnapshotVO snapshotVO = createSnapshotInDb(vol); SnapshotInfo snapshot = this.snapshotFactory.getSnapshot(snapshotVO.getId(), vol.getDataStore()); boolean result = false; + + StrategyPriority.sortStrategies(snapshotStrategies, snapshot); + for (SnapshotStrategy strategy : this.snapshotStrategies) { - if (strategy.canHandle(snapshot)) { + if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { snapshot = strategy.takeSnapshot(snapshot); result = true; } @@ -422,8 +425,11 @@ public class SnapshotTest extends CloudStackTestNGBase { SnapshotVO snapshotVO = createSnapshotInDb(vol); SnapshotInfo snapshot = this.snapshotFactory.getSnapshot(snapshotVO.getId(), vol.getDataStore()); SnapshotInfo newSnapshot = null; + + StrategyPriority.sortStrategies(snapshotStrategies, newSnapshot); + for (SnapshotStrategy strategy : this.snapshotStrategies) { - if (strategy.canHandle(snapshot)) { + if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { newSnapshot = strategy.takeSnapshot(snapshot); } } @@ -431,7 +437,7 @@ public class SnapshotTest extends CloudStackTestNGBase { // create another snapshot for (SnapshotStrategy strategy : this.snapshotStrategies) { - if (strategy.canHandle(snapshot)) { + if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { strategy.deleteSnapshot(newSnapshot.getId()); } } @@ -444,8 +450,11 @@ public class SnapshotTest extends CloudStackTestNGBase { SnapshotVO snapshotVO = createSnapshotInDb(vol); SnapshotInfo snapshot = this.snapshotFactory.getSnapshot(snapshotVO.getId(), vol.getDataStore()); boolean result = false; + + StrategyPriority.sortStrategies(snapshotStrategies, snapshot); + for (SnapshotStrategy strategy : this.snapshotStrategies) { - if (strategy.canHandle(snapshot)) { + if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { snapshot = strategy.takeSnapshot(snapshot); result = true; } @@ -467,15 +476,18 @@ public class SnapshotTest extends CloudStackTestNGBase { Mockito.when(epSelector.select(Matchers.any(DataObject.class), Matchers.any(DataObject.class))).thenReturn(remoteEp); } } - + @Test public void createSnapshot() throws InterruptedException, ExecutionException { VolumeInfo vol = createCopyBaseImage(); SnapshotVO snapshotVO = createSnapshotInDb(vol); SnapshotInfo snapshot = this.snapshotFactory.getSnapshot(snapshotVO.getId(), vol.getDataStore()); SnapshotInfo newSnapshot = null; + + StrategyPriority.sortStrategies(snapshotStrategies, newSnapshot); + for (SnapshotStrategy strategy : this.snapshotStrategies) { - if (strategy.canHandle(snapshot)) { + if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { newSnapshot = strategy.takeSnapshot(snapshot); } } @@ -487,7 +499,7 @@ public class SnapshotTest extends CloudStackTestNGBase { try { for (SnapshotStrategy strategy : this.snapshotStrategies) { - if (strategy.canHandle(snapshot)) { + if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { boolean res = strategy.deleteSnapshot(newSnapshot.getId()); Assert.assertTrue(res); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 aae4cde..6a874d6 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 @@ -26,6 +26,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.storage.command.CreateObjectAnswer; @@ -309,7 +310,7 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase { } @Override - public boolean canHandle(Snapshot snapshot) { - return true; + public StrategyPriority.Priority canHandle(Snapshot snapshot) { + return StrategyPriority.Priority.DEFAULT; } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 05b3e6b..74868c4 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 @@ -18,22 +18,28 @@ */ package org.apache.cloudstack.storage.motion; -import com.cloud.agent.api.to.VirtualMachineTO; -import com.cloud.host.Host; -import org.apache.cloudstack.engine.subsystem.api.storage.*; +import java.util.Map; + +import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; +import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; -import java.util.Map; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.host.Host; public class SimulatorDataMotionStrategy implements DataMotionStrategy { @Override - public boolean canHandle(DataObject srcData, DataObject destData) { - return true; + public Priority canHandle(DataObject srcData, DataObject destData) { + return Priority.HYPERVISOR; } @Override - public boolean canHandle(Map volumeMap, Host srcHost, Host destHost) { - return true; + public Priority canHandle(Map volumeMap, Host srcHost, Host destHost) { + return Priority.HYPERVISOR; } @Override http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 bdba61b..f5f23a2 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 @@ -21,12 +21,14 @@ package org.apache.cloudstack.storage.motion; import java.util.HashMap; import java.util.Map; + import javax.inject.Inject; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; @@ -62,17 +64,17 @@ public class VmwareStorageMotionStrategy implements DataMotionStrategy { @Inject VMInstanceDao instanceDao; @Override - public boolean canHandle(DataObject srcData, DataObject destData) { - return false; + public Priority canHandle(DataObject srcData, DataObject destData) { + return Priority.CANT_HANDLE; } @Override - public boolean canHandle(Map volumeMap, Host srcHost, Host destHost) { + public Priority 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 true; + return Priority.HYPERVISOR; } - return false; + return Priority.CANT_HANDLE; } @Override http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 b3ea5d5..c19480c 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 @@ -16,13 +16,6 @@ // under the License. package org.apache.cloudstack.storage.motion; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.anyLong; -import static org.mockito.Matchers.isA; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - import java.io.IOException; import java.util.HashMap; import java.util.Map; @@ -32,6 +25,7 @@ import javax.naming.ConfigurationException; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCallFuture; @@ -69,6 +63,14 @@ import com.cloud.utils.component.ComponentContext; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.isA; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(loader = AnnotationConfigContextLoader.class) public class VmwareStorageMotionStrategyTest { @@ -98,8 +100,8 @@ public class VmwareStorageMotionStrategyTest { when(srcHost.getHypervisorType()).thenReturn(HypervisorType.VMware); when(destHost.getHypervisorType()).thenReturn(HypervisorType.VMware); Map volumeMap = new HashMap(); - boolean canHandle = strategy.canHandle(volumeMap, srcHost, destHost); - assertTrue("The strategy is only supposed to handle vmware hosts", canHandle); + Priority canHandle = strategy.canHandle(volumeMap, srcHost, destHost); + assertTrue("The strategy is only supposed to handle vmware hosts", canHandle == Priority.HYPERVISOR); } @Test @@ -109,8 +111,8 @@ public class VmwareStorageMotionStrategyTest { when(srcHost.getHypervisorType()).thenReturn(HypervisorType.XenServer); when(destHost.getHypervisorType()).thenReturn(HypervisorType.XenServer); Map volumeMap = new HashMap(); - boolean canHandle = strategy.canHandle(volumeMap, srcHost, destHost); - assertFalse("The strategy is only supposed to handle vmware hosts", canHandle); + Priority canHandle = strategy.canHandle(volumeMap, srcHost, destHost); + assertFalse("The strategy is only supposed to handle vmware hosts", canHandle == Priority.HYPERVISOR); } @Test @@ -231,8 +233,8 @@ public class VmwareStorageMotionStrategyTest { @Configuration @ComponentScan(basePackageClasses = { VmwareStorageMotionStrategy.class }, - includeFilters = {@Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)}, - useDefaultFilters = false) + includeFilters = {@Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)}, + useDefaultFilters = false) public static class TestConfiguration extends SpringUtils.CloudStackTestConfiguration { @Bean http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 c796b69..8578a9a 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 @@ -24,18 +24,18 @@ import java.util.Map; import javax.inject.Inject; -import org.apache.log4j.Logger; -import org.springframework.stereotype.Component; - import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.log4j.Logger; +import org.springframework.stereotype.Component; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; @@ -71,18 +71,18 @@ public class XenServerStorageMotionStrategy implements DataMotionStrategy { @Inject VMInstanceDao instanceDao; @Override - public boolean canHandle(DataObject srcData, DataObject destData) { - return false; + public Priority canHandle(DataObject srcData, DataObject destData) { + return Priority.CANT_HANDLE; } @Override - public boolean canHandle(Map volumeMap, Host srcHost, Host destHost) { - boolean canHandle = false; + public Priority canHandle(Map volumeMap, Host srcHost, Host destHost) { if (srcHost.getHypervisorType() == HypervisorType.XenServer && destHost.getHypervisorType() == HypervisorType.XenServer) { - canHandle = true; + return Priority.HYPERVISOR; } - return canHandle; + + return Priority.CANT_HANDLE; } @Override http://git-wip-us.apache.org/repos/asf/cloudstack/blob/aad1cda7/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 a50d89c..dade983 100755 --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -40,6 +40,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; @@ -276,9 +278,11 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager, } } + StrategyPriority.sortStrategies(snapshotStrategies, snapshot); + SnapshotStrategy snapshotStrategy = null; for (SnapshotStrategy strategy : snapshotStrategies) { - if (strategy.canHandle(snapshot)) { + if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { snapshotStrategy = strategy; break; } @@ -378,7 +382,7 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager, <<<<<<< HEAD downloadSnapshotFromSwiftCommand cmd = new downloadSnapshotFromSwiftCommand(swift, secStore.getUri(), dcId, accountId, volumeId, parent, backupUuid, _backupsnapshotwait); ======= - DownloadSnapshotFromSwiftCommand cmd = new DownloadSnapshotFromSwiftCommand(swift, secondaryStoragePoolUrl, dcId, accountId, volumeId, parent, backupUuid, _backupsnapshotwait); + DownloadSnapshotFromSwiftCommand cmd = new DownloadSnapshotFromSwiftCommand(swift, secondaryStoragePoolUrl, dcId, accountId, volumeId, parent, backupUuid, _backupsnapshotwait); >>>>>>> master Answer answer = _agentMgr.sendToSSVM(dcId, cmd); if ((answer == null) || !answer.getResult()) { @@ -513,10 +517,12 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager, throw new InvalidParameterValueException("unable to find a snapshot with id " + snapshotId); } + StrategyPriority.sortStrategies(snapshotStrategies, snapshotCheck); + _accountMgr.checkAccess(caller, null, true, snapshotCheck); SnapshotStrategy snapshotStrategy = null; for (SnapshotStrategy strategy : snapshotStrategies) { - if (strategy.canHandle(snapshotCheck)) { + if (strategy.canHandle(snapshotCheck) != Priority.CANT_HANDLE) { snapshotStrategy = strategy; break; } @@ -707,8 +713,11 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager, for (SnapshotVO snapshot : snapshots) { SnapshotVO snap = _snapshotDao.findById(snapshot.getId()); SnapshotStrategy snapshotStrategy = null; + + StrategyPriority.sortStrategies(snapshotStrategies, snapshot); + for (SnapshotStrategy strategy : snapshotStrategies) { - if (strategy.canHandle(snap)) { + if (strategy.canHandle(snap) != Priority.CANT_HANDLE) { snapshotStrategy = strategy; break; } @@ -1038,9 +1047,11 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager, SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId, volume.getDataStore()); boolean processed = false; + StrategyPriority.sortStrategies(snapshotStrategies, snapshot); + try { for (SnapshotStrategy strategy : snapshotStrategies) { - if (strategy.canHandle(snapshot)) { + if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { processed = true; snapshot = strategy.takeSnapshot(snapshot); break;