From 0ed7ebd7e72988d091f129d37fd37acac3bd661f Mon Sep 17 00:00:00 2001 From: Chris Suich Date: Wed, 23 Oct 2013 15:17:24 -0400 Subject: [PATCH] Squashed & merged commit of the following: commit c9ee0d12e191e803fb341f3f96e95ca434a36f6c Author: Wei Zhou Date: Wed Oct 23 16:55:10 2013 +0200 CLOUDSTACK-4931, CLOUDSTACK-4937: setDetails to user VMs only (cherry picked from commit a94acc5a43aeaf5f18f1912e2653a92f6041a6e5) commit fe1586c71377bc6d219db2dcf088c40b65dd1fc4 Author: Anthony Xu Date: Tue Oct 22 11:20:27 2013 -0700 CLOUDSTACK-4649: vm sync tracks the pv driver version for xenserver Anthony commit 56a218f66eda540b4b4b04030ee71fc6863f8532 Author: Anthony Xu Date: Mon Oct 21 16:10:07 2013 -0700 CLOUDSTACK-4649: xs 6.1/6.2 introduce the new virtual platform, so there are two virtual platforms, windows PV driver version must match virtual platforms, this patch tracks PV driver versions in vm details and template details. Anthony commit 4e85d28c678a6f96b5b70d8d33fc60f9d1ea3df6 Author: Laszlo Hornyak Date: Mon Oct 21 21:17:33 2013 +0200 removed unused static field - s_httpClientManager was not used Signed-off-by: Laszlo Hornyak commit d4121fa26023db236f7396cea455ef090672ae9a Author: Chris Suich Date: Tue Oct 22 10:45:22 2013 -0400 Updated DataMotionServiceImpl and ApiResponseHelper based on review feedback. commit aaf026e1e4204d405bcda2ae4f1a01b1d0f7e7cb Author: Chris Suich Date: Thu Oct 17 14:27:12 2013 -0400 Added context to strategy sorting error responses Added TODOs for DRYing out pickStrategy() overloading commit a221f4aa3fb2ddc255bc35cf753f98f88f5bf44e Author: Chris Suich Date: Wed Oct 16 09:57:28 2013 -0400 Updated inefficient strategy sorting/selection Removed unnecessary canRevertSnapshot from PrimaryDataStoreDriver Other general cleaup and fixes from reviews commit 7d58949c6a1b7e853e891b59387a9620e8cd7a91 Author: Chris Suich Date: Mon Oct 14 14:01:22 2013 -0400 Added volume snapshot revert capability to SnapshotResponse Updated UI to hide/show snapshot revert action per snapshot Signed-off-by: Edison Su --- .../apache/cloudstack/api/ApiConstants.java | 4 +- .../api/response/SnapshotResponse.java | 25 ++- .../subsystem/api/storage/SnapshotInfo.java | 2 + .../api/storage/SnapshotStrategy.java | 9 +- .../api/storage/StrategyPriority.java | 96 ++++---- .../api/storage/StrategyPriorityTest.java | 90 +++++--- .../cloud/vm/VirtualMachineManagerImpl.java | 208 +++++++++--------- .../storage/motion/DataMotionServiceImpl.java | 38 ++-- .../test/FakePrimaryDataStoreDriver.java | 7 +- .../cloudstack/storage/test/SnapshotTest.java | 56 ++--- .../storage/snapshot/SnapshotObject.java | 39 +++- .../snapshot/XenserverSnapshotStrategy.java | 9 +- .../CloudStackPrimaryDataStoreDriverImpl.java | 49 +++-- .../SamplePrimaryDataStoreDriverImpl.java | 22 +- .../SolidfirePrimaryDataStoreDriver.java | 67 +++--- .../src/com/cloud/api/ApiResponseHelper.java | 22 +- .../storage/snapshot/SnapshotManagerImpl.java | 57 ++--- ui/scripts/storage.js | 2 +- 18 files changed, 443 insertions(+), 359 deletions(-) diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java index c75e6a0f9da..dbf668598dc 100755 --- a/api/src/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/org/apache/cloudstack/api/ApiConstants.java @@ -34,6 +34,7 @@ public class ApiConstants { public static final String BYTES_READ_RATE = "bytesreadrate"; public static final String BYTES_WRITE_RATE = "byteswriterate"; public static final String CATEGORY = "category"; + public static final String CAN_REVERT = "canrevert"; public static final String CERTIFICATE = "certificate"; public static final String PRIVATE_KEY = "privatekey"; public static final String DOMAIN_SUFFIX = "domainsuffix"; @@ -187,6 +188,7 @@ public class ApiConstants { public static final String REQUIRES_HVM = "requireshvm"; public static final String RESOURCE_TYPE = "resourcetype"; public static final String RESPONSE = "response"; + public static final String REVERTABLE = "revertable"; public static final String QUERY_FILTER = "queryfilter"; public static final String SCHEDULE = "schedule"; public static final String SCOPE = "scope"; @@ -249,7 +251,7 @@ public class ApiConstants { public static final String IS_VOLATILE = "isvolatile"; public static final String VOLUME_ID = "volumeid"; public static final String ZONE_ID = "zoneid"; - public static final String ZONE_NAME = "zonename"; + public static final String ZONE_NAME = "zonename"; public static final String NETWORK_TYPE = "networktype"; public static final String PAGE = "page"; public static final String PAGE_SIZE = "pagesize"; diff --git a/api/src/org/apache/cloudstack/api/response/SnapshotResponse.java b/api/src/org/apache/cloudstack/api/response/SnapshotResponse.java index e9cb109bf31..7c2b4a99770 100644 --- a/api/src/org/apache/cloudstack/api/response/SnapshotResponse.java +++ b/api/src/org/apache/cloudstack/api/response/SnapshotResponse.java @@ -26,18 +26,8 @@ import org.apache.cloudstack.api.EntityReference; import com.cloud.serializer.Param; import com.cloud.storage.Snapshot; import com.google.gson.annotations.SerializedName; -import com.cloud.serializer.Param; -import com.cloud.storage.Snapshot; -import com.google.gson.annotations.SerializedName; -import org.apache.cloudstack.api.ApiConstants; -import org.apache.cloudstack.api.BaseResponse; -import org.apache.cloudstack.api.EntityReference; - -import java.util.Date; -import java.util.List; @EntityReference(value=Snapshot.class) -@SuppressWarnings("unused") public class SnapshotResponse extends BaseResponse implements ControlledEntityResponse { @SerializedName(ApiConstants.ID) @Param(description = "ID of the snapshot") @@ -100,6 +90,9 @@ public class SnapshotResponse extends BaseResponse implements ControlledEntityRe @SerializedName(ApiConstants.TAGS) @Param(description="the list of resource tags associated with snapshot", responseObject = ResourceTagResponse.class) private List tags; + @SerializedName(ApiConstants.REVERTABLE) + @Param(description="indicates whether the underlying storage supports reverting the volume to this snapshot") + private boolean revertable; @Override public String getObjectId() { @@ -118,6 +111,7 @@ public class SnapshotResponse extends BaseResponse implements ControlledEntityRe return accountName; } + @Override public void setAccountName(String accountName) { this.accountName = accountName; } @@ -131,6 +125,7 @@ public class SnapshotResponse extends BaseResponse implements ControlledEntityRe this.domainId = domainId; } + @Override public void setDomainName(String domainName) { this.domainName = domainName; } @@ -180,8 +175,16 @@ public class SnapshotResponse extends BaseResponse implements ControlledEntityRe public void setZoneId(String zoneId) { this.zoneId = zoneId; } - + public void setTags(List tags) { this.tags = tags; } + + public boolean isRevertable() { + return revertable; + } + + public void setRevertable(boolean revertable) { + this.revertable = revertable; + } } diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java index 8d6b76010fe..a0ef7dd1273 100644 --- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java +++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java @@ -32,4 +32,6 @@ public interface SnapshotInfo extends DataObject, Snapshot { Long getDataCenterId(); ObjectInDataStoreStateMachine.State getStatus(); + + boolean isRevertable(); } 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 e4cecb6b156..3436d169cc9 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 @@ -19,6 +19,13 @@ package org.apache.cloudstack.engine.subsystem.api.storage; import com.cloud.storage.Snapshot; public interface SnapshotStrategy { + enum SnapshotOperation { + TAKE, + BACKUP, + DELETE, + REVERT + } + SnapshotInfo takeSnapshot(SnapshotInfo snapshot); SnapshotInfo backupSnapshot(SnapshotInfo snapshot); @@ -27,5 +34,5 @@ public interface SnapshotStrategy { boolean revertSnapshot(Long snapshotId); - StrategyPriority.Priority canHandle(Snapshot snapshot); + StrategyPriority.Priority canHandle(Snapshot snapshot, SnapshotOperation op); } 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 e930d103d11..05159ddbee9 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,11 +16,11 @@ // under the License. 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 org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation; + import com.cloud.host.Host; import com.cloud.storage.Snapshot; @@ -33,67 +33,49 @@ public class StrategyPriority { HIGHEST } - public static void sortStrategies(List strategies, Snapshot snapshot) { - Collections.sort(strategies, new SnapshotStrategyComparator(snapshot)); + 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; } - public static void sortStrategies(List strategies, DataObject srcData, DataObject destData) { - Collections.sort(strategies, new DataMotionStrategyComparator(srcData, destData)); + // 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 void sortStrategies(List strategies, Map volumeMap, Host srcHost, Host destHost) { - Collections.sort(strategies, new DataMotionStrategyHostComparator(volumeMap, srcHost, destHost)); - } + public static DataMotionStrategy pickStrategy(List strategies, Map volumeMap, Host srcHost, Host destHost) { + Priority highestPriority = Priority.CANT_HANDLE; + DataMotionStrategy strategyToUse = null; - static class SnapshotStrategyComparator implements Comparator { - - Snapshot snapshot; - - public SnapshotStrategyComparator(Snapshot snapshot) { - this.snapshot = snapshot; + for (DataMotionStrategy strategy : strategies) { + Priority priority = strategy.canHandle(volumeMap, srcHost, destHost); + if (priority.ordinal() > highestPriority.ordinal()) { + highestPriority = priority; + strategyToUse = strategy; + } } - @Override - public int compare(SnapshotStrategy o1, SnapshotStrategy o2) { - int i1 = o1.canHandle(snapshot).ordinal(); - int i2 = o2.canHandle(snapshot).ordinal(); - return Integer.compare(i2, 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 Integer.compare(i2, 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 Integer.compare(i2, i1); - } + return strategyToUse; } } 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 index 3d7527903b6..e18e6608c62 100644 --- 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 @@ -17,10 +17,10 @@ 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.SnapshotStrategy.SnapshotOperation; import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority; import org.junit.Test; @@ -43,22 +43,34 @@ public class StrategyPriorityTest { 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)); + 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); - strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy)); + SnapshotStrategy strategy = null; - StrategyPriority.sortStrategies(strategies, mock(Snapshot.class)); + 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); - 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)); + 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 @@ -76,15 +88,27 @@ public class StrategyPriorityTest { 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)); + DataMotionStrategy strategy = null; - StrategyPriority.sortStrategies(strategies, mock(DataObject.class), mock(DataObject.class)); + 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); - 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)); + 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 @@ -103,14 +127,26 @@ public class StrategyPriorityTest { 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)); + DataMotionStrategy strategy = null; - StrategyPriority.sortStrategies(strategies, mock(Map.class), mock(Host.class), mock(Host.class)); + 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); - 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)); + 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); } } diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java index c2242c7a34d..b4cb53f4430 100755 --- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -35,8 +35,6 @@ import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.log4j.Logger; - import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; @@ -50,8 +48,8 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.utils.identity.ManagementServerNode; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.utils.identity.ManagementServerNode; import org.apache.log4j.Logger; import com.cloud.agent.AgentManager; @@ -86,7 +84,6 @@ import com.cloud.agent.api.UnPlugNicCommand; import com.cloud.agent.api.to.DiskTO; import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.VirtualMachineTO; -import com.cloud.agent.api.to.VolumeTO; import com.cloud.agent.manager.Commands; import com.cloud.agent.manager.allocator.HostAllocator; import com.cloud.alert.AlertManager; @@ -283,20 +280,20 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac static final ConfigKey StartRetry = new ConfigKey(Integer.class, "start.retry", "Advanced", "10", "Number of times to retry create and start commands", true); static final ConfigKey VmOpWaitInterval = new ConfigKey("Advanced", Integer.class, "vm.op.wait.interval", "120", - "Time (in seconds) to wait before checking if a previous operation has succeeded", true); + "Time (in seconds) to wait before checking if a previous operation has succeeded", true); static final ConfigKey VmOpLockStateRetry = new ConfigKey("Advanced", Integer.class, "vm.op.lock.state.retry", "5", - "Times to retry locking the state of a VM for operations, -1 means forever", true); + "Times to retry locking the state of a VM for operations, -1 means forever", true); static final ConfigKey VmOpCleanupInterval = new ConfigKey("Advanced", Long.class, "vm.op.cleanup.interval", "86400", - "Interval to run the thread that cleans up the vm operations (in seconds)", false); + "Interval to run the thread that cleans up the vm operations (in seconds)", false); static final ConfigKey VmOpCleanupWait = new ConfigKey("Advanced", Long.class, "vm.op.cleanup.wait", "3600", - "Time (in seconds) to wait before cleanuping up any vm work items", true); + "Time (in seconds) to wait before cleanuping up any vm work items", true); static final ConfigKey VmOpCancelInterval = new ConfigKey("Advanced", Long.class, "vm.op.cancel.interval", "3600", - "Time (in seconds) to wait before cancelling a operation", false); + "Time (in seconds) to wait before cancelling a operation", false); static final ConfigKey VmDestroyForcestop = new ConfigKey("Advanced", Boolean.class, "vm.destroy.forcestop", "false", - "On destroy, force-stop takes this value ", true); + "On destroy, force-stop takes this value ", true); static final ConfigKey ClusterDeltaSyncInterval = new ConfigKey("Advanced", Integer.class, "sync.interval", "60", "Cluster Delta sync interval in seconds", - false); + false); ScheduledExecutorService _executor = null; @@ -312,8 +309,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Override @DB public void allocate(String vmInstanceName, VirtualMachineTemplate template, ServiceOffering serviceOffering, Pair rootDiskOffering, - LinkedHashMap dataDiskOfferings, LinkedHashMap auxiliaryNetworks, DeploymentPlan plan, - HypervisorType hyperType) throws InsufficientCapacityException { + LinkedHashMap dataDiskOfferings, LinkedHashMap auxiliaryNetworks, DeploymentPlan plan, + HypervisorType hyperType) throws InsufficientCapacityException { VMInstanceVO vm = _vmDao.findVMByInstanceName(vmInstanceName); Account owner = _entityMgr.findById(Account.class, vm.getAccountId()); @@ -372,7 +369,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Override public void allocate(String vmInstanceName, VirtualMachineTemplate template, ServiceOffering serviceOffering, LinkedHashMap networks, - DeploymentPlan plan, HypervisorType hyperType) throws InsufficientCapacityException { + DeploymentPlan plan, HypervisorType hyperType) throws InsufficientCapacityException { allocate(vmInstanceName, template, serviceOffering, new Pair(serviceOffering, null), null, networks, plan, hyperType); } @@ -552,7 +549,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @DB protected Ternary changeToStartState(VirtualMachineGuru vmGuru, VMInstanceVO vm, User caller, Account account) - throws ConcurrentOperationException { + throws ConcurrentOperationException { long vmId = vm.getId(); ItWorkVO work = new ItWorkVO(UUID.randomUUID().toString(), _nodeId, State.Starting, vm.getType(), vm.getId()); @@ -645,13 +642,13 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Override public void advanceStart(String vmUuid, Map params) throws InsufficientCapacityException, ConcurrentOperationException, - ResourceUnavailableException { + ResourceUnavailableException { advanceStart(vmUuid, params, null); } @Override public void advanceStart(String vmUuid, Map params, DeploymentPlan planToDeploy) throws InsufficientCapacityException, - ConcurrentOperationException, ResourceUnavailableException { + ConcurrentOperationException, ResourceUnavailableException { CallContext cctxt = CallContext.current(); Account account = cctxt.getCallingAccount(); User caller = cctxt.getCallingUser(); @@ -680,10 +677,10 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (planToDeploy != null && planToDeploy.getDataCenterId() != 0) { if (s_logger.isDebugEnabled()) { s_logger.debug("advanceStart: DeploymentPlan is provided, using dcId:" + planToDeploy.getDataCenterId() + ", podId: " + planToDeploy.getPodId() + ", clusterId: " + - planToDeploy.getClusterId() + ", hostId: " + planToDeploy.getHostId() + ", poolId: " + planToDeploy.getPoolId()); + planToDeploy.getClusterId() + ", hostId: " + planToDeploy.getHostId() + ", poolId: " + planToDeploy.getPoolId()); } plan = new DataCenterDeployment(planToDeploy.getDataCenterId(), planToDeploy.getPodId(), planToDeploy.getClusterId(), planToDeploy.getHostId(), - planToDeploy.getPoolId(), planToDeploy.getPhysicalNetworkId(), ctx); + planToDeploy.getPoolId(), planToDeploy.getPhysicalNetworkId(), ctx); } HypervisorGuru hvGuru = _hvGuruMgr.getGuru(vm.getHypervisorType()); @@ -741,20 +738,20 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac // planner if (s_logger.isDebugEnabled()) { s_logger.debug("Cannot satisfy the deployment plan passed in since the ready Root volume is in different cluster. volume's cluster: " + - rootVolClusterId + ", cluster specified: " + clusterIdSpecified); + rootVolClusterId + ", cluster specified: " + clusterIdSpecified); } throw new ResourceUnavailableException( - "Root volume is ready in different cluster, Deployment plan provided cannot be satisfied, unable to create a deployment for " + vm, - Cluster.class, clusterIdSpecified); + "Root volume is ready in different cluster, Deployment plan provided cannot be satisfied, unable to create a deployment for " + vm, + Cluster.class, clusterIdSpecified); } } plan = new DataCenterDeployment(planToDeploy.getDataCenterId(), planToDeploy.getPodId(), planToDeploy.getClusterId(), planToDeploy.getHostId(), - vol.getPoolId(), null, ctx); + vol.getPoolId(), null, ctx); } else { plan = new DataCenterDeployment(rootVolDcId, rootVolPodId, rootVolClusterId, null, vol.getPoolId(), null, ctx); if (s_logger.isDebugEnabled()) { s_logger.debug(vol + " is READY, changing deployment plan to use this pool's dcId: " + rootVolDcId + " , podId: " + rootVolPodId + - " , and clusterId: " + rootVolClusterId); + " , and clusterId: " + rootVolClusterId); } planChangedByVolume = true; } @@ -781,7 +778,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac continue; } throw new InsufficientServerCapacityException("Unable to create a deployment for " + vmProfile, DataCenter.class, plan.getDataCenterId(), - areAffinityGroupsAssociated(vmProfile)); + areAffinityGroupsAssociated(vmProfile)); } if (dest != null) { @@ -796,7 +793,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac ClusterDetailsVO cluster_detail_ram = _clusterDetailsDao.findDetail(cluster_id, "memoryOvercommitRatio"); //storing the value of overcommit in the vm_details table for doing a capacity check in case the cluster overcommit ratio is changed. if (_uservmDetailsDao.findDetail(vm.getId(), "cpuOvercommitRatio") == null && - ((Float.parseFloat(cluster_detail_cpu.getValue()) > 1f || Float.parseFloat(cluster_detail_ram.getValue()) > 1f))) { + ((Float.parseFloat(cluster_detail_cpu.getValue()) > 1f || Float.parseFloat(cluster_detail_ram.getValue()) > 1f))) { UserVmDetailVO vmDetail_cpu = new UserVmDetailVO(vm.getId(), "cpuOvercommitRatio", cluster_detail_cpu.getValue()); UserVmDetailVO vmDetail_ram = new UserVmDetailVO(vm.getId(), "memoryOvercommitRatio", cluster_detail_ram.getValue()); _uservmDetailsDao.persist(vmDetail_cpu); @@ -867,7 +864,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } if (vmGuru.finalizeStart(vmProfile, destHostId, cmds, ctx)) { syncDiskChainChange(startAnswer); - + if (!changeState(vm, Event.OperationSucceeded, destHostId, work, Step.Done)) { throw new ConcurrentOperationException("Unable to transition to a new state."); } @@ -884,7 +881,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac StopCommand cmd = new StopCommand(vm, getExecuteInSequence()); StopAnswer answer = (StopAnswer)_agentMgr.easySend(destHostId, cmd); - if ( answer != null ) { + if ( answer != null ) { String hypervisortoolsversion = answer.getHypervisorToolsVersion(); if (hypervisortoolsversion != null) { if (vm.getType() == VirtualMachine.Type.User) { @@ -969,17 +966,17 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac throw new CloudRuntimeException("Unable to start instance '" + vm.getHostName() + "' (" + vm.getUuid() + "), see management server log for details"); } } - + private void syncDiskChainChange(StartAnswer answer) { - VirtualMachineTO vmSpec = answer.getVirtualMachine(); - - for(DiskTO disk : vmSpec.getDisks()) { - if(disk.getType() != Volume.Type.ISO) { - VolumeObjectTO vol = (VolumeObjectTO)disk.getData(); - - volumeMgr.updateVolumeDiskChain(vol.getId(), vol.getPath(), vol.getChainInfo()); - } - } + VirtualMachineTO vmSpec = answer.getVirtualMachine(); + + for(DiskTO disk : vmSpec.getDisks()) { + if(disk.getType() != Volume.Type.ISO) { + VolumeObjectTO vol = (VolumeObjectTO)disk.getData(); + + volumeMgr.updateVolumeDiskChain(vol.getId(), vol.getPath(), vol.getChainInfo()); + } + } } @Override @@ -1563,8 +1560,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac _networkMgr.rollbackNicForMigration(vmSrc, profile); _alertMgr.sendAlert(alertType, fromHost.getDataCenterId(), fromHost.getPodId(), "Unable to migrate vm " + vm.getInstanceName() + " from host " + - fromHost.getName() + " in zone " + dest.getDataCenter().getName() + " and pod " + - dest.getPod().getName(), "Migrate Command failed. Please check logs."); + fromHost.getName() + " in zone " + dest.getDataCenter().getName() + " and pod " + + dest.getPod().getName(), "Migrate Command failed. Please check logs."); try { _agentMgr.send(dstHostId, new Commands(cleanup(vm)), null); } catch (AgentUnavailableException ae) { @@ -1597,8 +1594,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (_poolHostDao.findByPoolHost(pool.getId(), host.getId()) == null || pool.isLocal() != diskOffering.getUseLocalStorage()) { // Cannot find a pool for the volume. Throw an exception. throw new CloudRuntimeException("Cannot migrate volume " + volume + " to storage pool " + pool + " while migrating vm to host " + host + - ". Either the pool is not accessible from the " + - "host or because of the offering with which the volume is created it cannot be placed on " + "the given pool."); + ". Either the pool is not accessible from the " + + "host or because of the offering with which the volume is created it cannot be placed on " + "the given pool."); } else if (pool.getId() == currentPool.getId()) { // If the pool to migrate too is the same as current pool, remove the volume from the list of // volumes to be migrated. @@ -1630,7 +1627,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (!currentPoolAvailable && !volumeToPool.containsKey(volume)) { // Cannot find a pool for the volume. Throw an exception. throw new CloudRuntimeException("Cannot find a storage pool which is available for volume " + volume + " while migrating virtual machine " + - profile.getVirtualMachine() + " to host " + host); + profile.getVirtualMachine() + " to host " + host); } } } @@ -1666,7 +1663,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Override public void migrateWithStorage(String vmUuid, long srcHostId, long destHostId, Map volumeToPool) throws ResourceUnavailableException, - ConcurrentOperationException { + ConcurrentOperationException { VMInstanceVO vm = _vmDao.findByUuid(vmUuid); HostVO srcHost = _hostDao.findById(srcHostId); @@ -1686,7 +1683,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac // a vm and not migrating a vm with storage. if (volumeToPool.isEmpty()) { throw new InvalidParameterValueException("Migration of the vm " + vm + "from host " + srcHost + " to destination host " + destHost + - " doesn't involve migrating the volumes."); + " doesn't involve migrating the volumes."); } short alertType = AlertManager.ALERT_TYPE_USERVM_MIGRATE; @@ -1739,8 +1736,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (!migrated) { s_logger.info("Migration was unsuccessful. Cleaning up: " + vm); _alertMgr.sendAlert(alertType, srcHost.getDataCenterId(), srcHost.getPodId(), "Unable to migrate vm " + vm.getInstanceName() + " from host " + srcHost.getName() + - " in zone " + dc.getName() + " and pod " + dc.getName(), - "Migrate Command failed. Please check logs."); + " in zone " + dc.getName() + " and pod " + dc.getName(), + "Migrate Command failed. Please check logs."); try { _agentMgr.send(destHostId, new Commands(cleanup(vm.getInstanceName())), null); stateTransitTo(vm, Event.OperationFailed, srcHostId); @@ -1908,7 +1905,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Override public void advanceReboot(String vmUuid, Map params) throws InsufficientCapacityException, ConcurrentOperationException, - ResourceUnavailableException { + ResourceUnavailableException { VMInstanceVO vm = _vmDao.findByUuid(vmUuid); DataCenter dc = _entityMgr.findById(DataCenter.class, vm.getDataCenterId()); @@ -1957,7 +1954,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac // sync VM Snapshots related transient states List vmSnapshotsInTrasientStates = _vmSnapshotDao.listByInstanceId(vm.getId(), VMSnapshot.State.Expunging, VMSnapshot.State.Reverting, - VMSnapshot.State.Creating); + VMSnapshot.State.Creating); if (vmSnapshotsInTrasientStates.size() > 1) { s_logger.info("Found vm " + vm.getInstanceName() + " with VM snapshots in transient states, needs to sync VM snapshot state"); if (!_vmSnapshotMgr.syncVMSnapshot(vm, hostId)) { @@ -2071,7 +2068,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac public void fullSync(final long clusterId, Map> newStates) { if (newStates==null) - return; + return; Map infos = convertToInfos(newStates); Set set_vms = Collections.synchronizedSet(new HashSet()); set_vms.addAll(_vmDao.listByClusterId(clusterId)); @@ -2082,7 +2079,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac // sync VM Snapshots related transient states List vmSnapshotsInExpungingStates = _vmSnapshotDao.listByInstanceId(vm.getId(), VMSnapshot.State.Expunging, VMSnapshot.State.Creating, - VMSnapshot.State.Reverting); + VMSnapshot.State.Reverting); if (vmSnapshotsInExpungingStates.size() > 0) { s_logger.info("Found vm " + vm.getInstanceName() + " in state. " + vm.getState() + ", needs to sync VM snapshot state"); Long hostId = null; @@ -2100,9 +2097,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } if ((info == null && (vm.getState() == State.Running || vm.getState() == State.Starting)) || - (info != null && (info.state == State.Running && vm.getState() == State.Starting))) { + (info != null && (info.state == State.Running && vm.getState() == State.Starting))) { s_logger.info("Found vm " + vm.getInstanceName() + " in inconsistent state. " + vm.getState() + " on CS while " + (info == null ? "Stopped" : "Running") + - " on agent"); + " on agent"); info = new AgentVmInfo(vm.getInstanceName(), vm, State.Stopped); // Bug 13850- grab outstanding work item if any for this VM state so that we mark it as DONE after we change VM state, else it will remain pending @@ -2139,7 +2136,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac e.printStackTrace(); } } else if (info != null && - (vm.getState() == State.Stopped || vm.getState() == State.Stopping || vm.isRemoved() || vm.getState() == State.Destroyed || vm.getState() == State.Expunging)) { + (vm.getState() == State.Stopped || vm.getState() == State.Stopping || vm.isRemoved() || vm.getState() == State.Destroyed || vm.getState() == State.Expunging)) { Host host = _hostDao.findByGuid(info.getHostUuid()); if (host != null) { s_logger.warn("Stopping a VM which is stopped/stopping/destroyed/expunging " + info.name); @@ -2158,19 +2155,19 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } } else - // host id can change - if (info != null && vm.getState() == State.Running) { - // check for host id changes - Host host = _hostDao.findByGuid(info.getHostUuid()); - if (host != null && (vm.getHostId() == null || host.getId() != vm.getHostId())) { - s_logger.info("Found vm " + vm.getInstanceName() + " with inconsistent host in db, new host is " + host.getId()); - try { - stateTransitTo(vm, VirtualMachine.Event.AgentReportMigrated, host.getId()); - } catch (NoTransitionException e) { - s_logger.warn(e.getMessage()); + // host id can change + if (info != null && vm.getState() == State.Running) { + // check for host id changes + Host host = _hostDao.findByGuid(info.getHostUuid()); + if (host != null && (vm.getHostId() == null || host.getId() != vm.getHostId())) { + s_logger.info("Found vm " + vm.getInstanceName() + " with inconsistent host in db, new host is " + host.getId()); + try { + stateTransitTo(vm, VirtualMachine.Event.AgentReportMigrated, host.getId()); + } catch (NoTransitionException e) { + s_logger.warn(e.getMessage()); + } } } - } /* else if(info == null && vm.getState() == State.Stopping) { //Handling CS-13376 s_logger.warn("Marking the VM as Stopped as it was still stopping on the CS" +vm.getName()); vm.setState(State.Stopped); // Setting the VM as stopped on the DB and clearing it from the host @@ -2213,13 +2210,13 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac VMInstanceVO vm = _vmDao.findVMByInstanceName(name); if (vm != null) { map.put(vm.getId(), new AgentVmInfo(entry.getKey(), vm, entry.getValue().second(), - entry.getValue().first(), entry.getValue().third())); + entry.getValue().first(), entry.getValue().third())); is_alien_vm = false; } // alien VMs if (is_alien_vm) { - map.put(alien_vm_count--, new AgentVmInfo(entry.getKey(), null, entry.getValue().second(), - entry.getValue().first(), entry.getValue().third())); + map.put(alien_vm_count--, new AgentVmInfo(entry.getKey(), null, entry.getValue().second(), + entry.getValue().first(), entry.getValue().third())); s_logger.warn("Found an alien VM " + entry.getKey()); } } @@ -2297,8 +2294,17 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac String hostDesc = "name: " + hostVO.getName() + " (id:" + hostVO.getId() + "), availability zone: " + dcVO.getName() + ", pod: " + podVO.getName(); _alertMgr.sendAlert(alertType, vm.getDataCenterId(), vm.getPodIdToDeployIn(), "VM (name: " + vm.getInstanceName() + ", id: " + vm.getId() + ") stopped on host " + - hostDesc + " due to storage failure", - "Virtual Machine " + vm.getInstanceName() + " (id: " + vm.getId() + ") running on host [" + vm.getHostId() + "] stopped due to storage failure."); + hostDesc + " due to storage failure", + "Virtual Machine " + vm.getInstanceName() + " (id: " + vm.getId() + ") running on host [" + vm.getHostId() + "] stopped due to storage failure."); + } + // track hypervsion tools version + if( info.hvtoolsversion != null && !info.hvtoolsversion.isEmpty() ) { + if (vm.getType() == VirtualMachine.Type.User) { + UserVmVO userVm = _userVmDao.findById(vm.getId()); + _userVmDao.loadDetails(userVm); + userVm.setDetail("hypervisortoolsversion", info.hvtoolsversion); + _userVmDao.saveDetails(userVm); + } } // track hypervsion tools version if( info.hvtoolsversion != null && !info.hvtoolsversion.isEmpty() ) { @@ -2314,7 +2320,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (serverState == State.Starting) { if (vm.getHostId() != null && vm.getHostId() != hostId) { s_logger.info("CloudStack is starting VM on host " + vm.getHostId() + ", but status report comes from a different host " + hostId + - ", skip status sync for vm: " + vm.getInstanceName()); + ", skip status sync for vm: " + vm.getInstanceName()); return null; } } @@ -2339,7 +2345,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (serverState == State.Starting) { if (vm.getHostId() != null && vm.getHostId() != hostId) { s_logger.info("CloudStack is starting VM on host " + vm.getHostId() + ", but status report comes from a different host " + hostId + - ", skip status sync for vm: " + vm.getInstanceName()); + ", skip status sync for vm: " + vm.getInstanceName()); return null; } } @@ -2355,7 +2361,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (vm.getHostId() == null || hostId != vm.getHostId()) { if (s_logger.isDebugEnabled()) { s_logger.debug("detected host change when VM " + vm + " is at running state, VM could be live-migrated externally from host " + vm.getHostId() + - " to host " + hostId); + " to host " + hostId); } stateTransitTo(vm, VirtualMachine.Event.AgentReportMigrated, hostId); @@ -2453,7 +2459,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } private void ensureVmRunningContext(long hostId, VMInstanceVO vm, Event cause) throws OperationTimedoutException, ResourceUnavailableException, NoTransitionException, - InsufficientAddressCapacityException { + InsufficientAddressCapacityException { VirtualMachineGuru vmGuru = getVmGuru(vm); s_logger.debug("VM state is starting on full sync so updating it to running"); @@ -2482,7 +2488,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac for (NicVO nic : nics) { Network network = _networkModel.getNetwork(nic.getNetworkId()); NicProfile nicProfile = new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), null, _networkModel.isSecurityGroupSupportedInNetwork(network), - _networkModel.getNetworkTag(profile.getHypervisorType(), network)); + _networkModel.getNetworkTag(profile.getHypervisorType(), network)); profile.addNic(nicProfile); } @@ -2685,13 +2691,13 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac this.vm = vm; this.hostUuid = host; this.hvtoolsversion= hvtoolsversion; - + } - + public AgentVmInfo(String name, VMInstanceVO vm, State state, String host) { - this(name, vm, state, host, null); + this(name, vm, state, host, null); } - + public AgentVmInfo(String name, VMInstanceVO vm, State state) { this(name, vm, state, null, null); } @@ -2699,7 +2705,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac public String getHostUuid() { return hostUuid; } - + public String getHvtoolsversion() { return hvtoolsversion; } @@ -2721,7 +2727,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (!(vmInstance.getState().equals(State.Stopped) || vmInstance.getState().equals(State.Running))) { s_logger.warn("Unable to upgrade virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState()); throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + " " + " in state " + vmInstance.getState() + - "; make sure the virtual machine is stopped/running"); + "; make sure the virtual machine is stopped/running"); } // Check if the service offering being upgraded to is what the VM is already running with @@ -2731,7 +2737,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } throw new InvalidParameterValueException("Not upgrading vm " + vmInstance.toString() + " since it already " + "has the requested service offering (" + - newServiceOffering.getName() + ")"); + newServiceOffering.getName() + ")"); } ServiceOfferingVO currentServiceOffering = _offeringDao.findByIdIncludingRemoved(vmInstance.getServiceOfferingId()); @@ -2749,8 +2755,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac // offering if (currentServiceOffering.getUseLocalStorage() != newServiceOffering.getUseLocalStorage()) { throw new InvalidParameterValueException("Unable to upgrade virtual machine " + vmInstance.toString() + - ", cannot switch between local storage and shared storage service offerings. Current offering " + "useLocalStorage=" + - currentServiceOffering.getUseLocalStorage() + ", target offering useLocalStorage=" + newServiceOffering.getUseLocalStorage()); + ", cannot switch between local storage and shared storage service offerings. Current offering " + "useLocalStorage=" + + currentServiceOffering.getUseLocalStorage() + ", target offering useLocalStorage=" + newServiceOffering.getUseLocalStorage()); } // if vm is a system vm, check if it is a system service offering, if yes return with error as it cannot be used for user vms @@ -2761,7 +2767,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac // Check that there are enough resources to upgrade the service offering if (!isVirtualMachineUpgradable(vmInstance, newServiceOffering)) { throw new InvalidParameterValueException("Unable to upgrade virtual machine, not enough resources available " + "for an offering of " + newServiceOffering.getCpu() + - " cpu(s) at " + newServiceOffering.getSpeed() + " Mhz, and " + newServiceOffering.getRamSize() + " MB of memory"); + " cpu(s) at " + newServiceOffering.getSpeed() + " Mhz, and " + newServiceOffering.getRamSize() + " MB of memory"); } // Check that the service offering being upgraded to has all the tags of the current service offering @@ -2769,8 +2775,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac List newTags = StringUtils.csvTagsToList(newServiceOffering.getTags()); if (!newTags.containsAll(currentTags)) { throw new InvalidParameterValueException("Unable to upgrade virtual machine; the new service offering " + "does not have all the tags of the " + - "current service offering. Current service offering tags: " + currentTags + "; " + "new service " + "offering tags: " + - newTags); + "current service offering. Current service offering tags: " + currentTags + "; " + "new service " + "offering tags: " + + newTags); } } @@ -2787,7 +2793,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Override public NicProfile addVmToNetwork(VirtualMachine vm, Network network, NicProfile requested) throws ConcurrentOperationException, ResourceUnavailableException, - InsufficientCapacityException { + InsufficientCapacityException { CallContext cctx = CallContext.current(); s_logger.debug("Adding vm " + vm + " to network " + network + "; requested nic profile " + requested); @@ -2823,7 +2829,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac long isDefault = (nic.isDefaultNic()) ? 1 : 0; // insert nic's Id into DB as resource_name UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NETWORK_OFFERING_ASSIGN, vmVO.getAccountId(), vmVO.getDataCenterId(), vmVO.getId(), - Long.toString(nic.getId()), network.getNetworkOfferingId(), null, isDefault, VirtualMachine.class.getName(), vmVO.getUuid()); + Long.toString(nic.getId()), network.getNetworkOfferingId(), null, isDefault, VirtualMachine.class.getName(), vmVO.getUuid()); return nic; } else { s_logger.warn("Failed to plug nic to the vm " + vm + " in network " + network); @@ -2879,7 +2885,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } NicProfile nicProfile = new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), _networkModel.getNetworkRate(network.getId(), vm.getId()), - _networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag(vmProfile.getVirtualMachine().getHypervisorType(), network)); + _networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag(vmProfile.getVirtualMachine().getHypervisorType(), network)); //1) Unplug the nic if (vm.getState() == State.Running) { @@ -2890,7 +2896,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac s_logger.debug("Nic is unplugged successfully for vm " + vm + " in network " + network); long isDefault = (nic.isDefaultNic()) ? 1 : 0; UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NETWORK_OFFERING_REMOVE, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), Long.toString(nic.getId()), - network.getNetworkOfferingId(), null, isDefault, VirtualMachine.class.getName(), vm.getUuid()); + network.getNetworkOfferingId(), null, isDefault, VirtualMachine.class.getName(), vm.getUuid()); } else { s_logger.warn("Failed to unplug nic for the vm " + vm + " from network " + network); return false; @@ -2962,7 +2968,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac try { NicProfile nicProfile = new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), _networkModel.getNetworkRate(network.getId(), vm.getId()), - _networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag(vmProfile.getVirtualMachine().getHypervisorType(), network)); + _networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag(vmProfile.getVirtualMachine().getHypervisorType(), network)); //1) Unplug the nic if (vm.getState() == State.Running) { @@ -2999,7 +3005,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Override public void findHostAndMigrate(String vmUuid, Long newSvcOfferingId, ExcludeList excludes) throws InsufficientCapacityException, ConcurrentOperationException, - ResourceUnavailableException { + ResourceUnavailableException { VMInstanceVO vm = _vmDao.findByUuid(vmUuid); if (vm == null) { @@ -3184,8 +3190,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac s_logger.info("Migration was unsuccessful. Cleaning up: " + vm); _alertMgr.sendAlert(alertType, fromHost.getDataCenterId(), fromHost.getPodId(), "Unable to migrate vm " + vm.getInstanceName() + " from host " + - fromHost.getName() + " in zone " + dest.getDataCenter().getName() + " and pod " + - dest.getPod().getName(), "Migrate Command failed. Please check logs."); + fromHost.getName() + " in zone " + dest.getDataCenter().getName() + " and pod " + + dest.getPod().getName(), "Migrate Command failed. Please check logs."); try { _agentMgr.send(dstHostId, new Commands(cleanup(vm.getInstanceName())), null); } catch (AgentUnavailableException ae) { @@ -3205,7 +3211,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } public boolean plugNic(Network network, NicTO nic, VirtualMachineTO vm, ReservationContext context, DeployDestination dest) throws ConcurrentOperationException, - ResourceUnavailableException, InsufficientCapacityException { + ResourceUnavailableException, InsufficientCapacityException { boolean result = true; VMInstanceVO router = _vmDao.findById(vm.getId()); @@ -3228,14 +3234,14 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac s_logger.warn("Unable to apply PlugNic, vm " + router + " is not in the right state " + router.getState()); throw new ResourceUnavailableException("Unable to apply PlugNic on the backend," + " vm " + vm + " is not in the right state", DataCenter.class, - router.getDataCenterId()); + router.getDataCenterId()); } return result; } public boolean unplugNic(Network network, NicTO nic, VirtualMachineTO vm, ReservationContext context, DeployDestination dest) throws ConcurrentOperationException, - ResourceUnavailableException { + ResourceUnavailableException { boolean result = true; VMInstanceVO router = _vmDao.findById(vm.getId()); @@ -3261,7 +3267,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac s_logger.warn("Unable to apply unplug nic, Vm " + router + " is not in the right state " + router.getState()); throw new ResourceUnavailableException("Unable to apply unplug nic on the backend," + " vm " + router + " is not in the right state", DataCenter.class, - router.getDataCenterId()); + router.getDataCenterId()); } return result; @@ -3269,7 +3275,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Override public VMInstanceVO reConfigureVm(String vmUuid, ServiceOffering oldServiceOffering, boolean reconfiguringOnExistingHost) throws ResourceUnavailableException, - ConcurrentOperationException { + ConcurrentOperationException { VMInstanceVO vm = _vmDao.findByUuid(vmUuid); long newServiceofferingId = vm.getServiceOfferingId(); @@ -3280,7 +3286,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac Float cpuOvercommitRatio = CapacityManager.CpuOverprovisioningFactor.valueIn(hostVo.getClusterId()); long minMemory = (long)(newServiceOffering.getRamSize() / memoryOvercommitRatio); ScaleVmCommand reconfigureCmd = new ScaleVmCommand(vm.getInstanceName(), newServiceOffering.getCpu(), (int)(newServiceOffering.getSpeed() / cpuOvercommitRatio), - newServiceOffering.getSpeed(), minMemory * 1024L * 1024L, newServiceOffering.getRamSize() * 1024L * 1024L, newServiceOffering.getLimitCpuUse()); + newServiceOffering.getSpeed(), minMemory * 1024L * 1024L, newServiceOffering.getRamSize() * 1024L * 1024L, newServiceOffering.getLimitCpuUse()); Long dstHostId = vm.getHostId(); ItWorkVO work = new ItWorkVO(UUID.randomUUID().toString(), _nodeId, State.Running, vm.getType(), vm.getId()); 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 2d3132095e3..213b47a9791 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,6 +18,7 @@ */ package org.apache.cloudstack.storage.motion; +import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -29,13 +30,13 @@ 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 com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.host.Host; +import com.cloud.utils.StringUtils; import com.cloud.utils.exception.CloudRuntimeException; @Component @@ -57,30 +58,35 @@ public class DataMotionServiceImpl implements DataMotionService { return; } - StrategyPriority.sortStrategies(strategies, srcData, destData); - - for (DataMotionStrategy strategy : strategies) { - if (strategy.canHandle(srcData, destData) != Priority.CANT_HANDLE) { - strategy.copyAsync(srcData, destData, callback); - return; - } + // TODO DRY this out when the overloaded methods are DRYed out + DataMotionStrategy strategy = StrategyPriority.pickStrategy(strategies, srcData, destData); + if (strategy == null) { + throw new CloudRuntimeException("Can't find strategy to move data. "+ + "Source: "+srcData.getType().name()+" '"+srcData.getUuid()+ + ", Destination: "+destData.getType().name()+" '"+destData.getUuid()+"'"); } - throw new CloudRuntimeException("can't find strategy to move data"); + + strategy.copyAsync(srcData, destData, callback); } @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) != Priority.CANT_HANDLE) { - strategy.copyAsync(volumeMap, vmTo, srcHost, destHost, callback); - return; + // TODO DRY this out when the overloaded methods are DRYed out + DataMotionStrategy strategy = StrategyPriority.pickStrategy(strategies, volumeMap, srcHost, destHost); + if (strategy == null) { + List volumeIds = new LinkedList(); + for (final VolumeInfo volumeInfo : volumeMap.keySet()) { + volumeIds.add(volumeInfo.getUuid()); } + + throw new CloudRuntimeException("Can't find strategy to move data. "+ + "Source Host: "+srcHost.getName()+", Destination Host: "+destHost.getName()+ + ", Volume UUIDs: "+StringUtils.join(volumeIds, ",")); } - throw new CloudRuntimeException("can't find strategy to move data"); + + strategy.copyAsync(volumeMap, vmTo, srcHost, destHost, callback); } public void setStrategies(List strategies) { diff --git a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/FakePrimaryDataStoreDriver.java b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/FakePrimaryDataStoreDriver.java index 810afd11577..de64b8f5ca2 100644 --- a/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/FakePrimaryDataStoreDriver.java +++ b/engine/storage/integration-test/test/org/apache/cloudstack/storage/test/FakePrimaryDataStoreDriver.java @@ -18,8 +18,8 @@ */ package org.apache.cloudstack.storage.test; -import com.cloud.agent.api.to.DataStoreTO; -import com.cloud.agent.api.to.DataTO; +import java.util.UUID; + import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; @@ -33,7 +33,8 @@ import org.apache.cloudstack.storage.command.CommandResult; import org.apache.cloudstack.storage.command.CreateObjectAnswer; import org.apache.cloudstack.storage.to.SnapshotObjectTO; -import java.util.UUID; +import com.cloud.agent.api.to.DataStoreTO; +import com.cloud.agent.api.to.DataTO; public class FakePrimaryDataStoreDriver implements PrimaryDataStoreDriver { boolean snapshotResult = true; 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 81f77d62429..a46ebd80e50 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 @@ -39,13 +39,16 @@ 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.SnapshotStrategy.SnapshotOperation; +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.TemplateDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService; -import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService.VolumeApiResult; import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.storage.LocalHostEndpoint; @@ -281,7 +284,7 @@ public class SnapshotTest extends CloudStackTestNGBase { Mockito.when(epSelector.select(Matchers.any(DataObject.class))).thenReturn(remoteEp); Mockito.when(epSelector.select(Matchers.any(DataStore.class))).thenReturn(remoteEp); Mockito.when(hyGuruMgr.getGuruProcessedCommandTargetHost(Matchers.anyLong(), Matchers.any(Command.class))) - .thenReturn(this.host.getId()); + .thenReturn(this.host.getId()); } @@ -367,10 +370,10 @@ public class SnapshotTest extends CloudStackTestNGBase { result = future.get(); Assert.assertTrue(result.isSuccess()); return result.getVolume(); - + } - + private VMTemplateVO createTemplateInDb() { VMTemplateVO image = new VMTemplateVO(); @@ -401,13 +404,10 @@ public class SnapshotTest extends CloudStackTestNGBase { 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) != Priority.CANT_HANDLE) { - snapshot = strategy.takeSnapshot(snapshot); - result = true; - } + SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.TAKE); + if (snapshotStrategy != null) { + snapshot = snapshotStrategy.takeSnapshot(snapshot); + result = true; } AssertJUnit.assertTrue(result); @@ -426,18 +426,15 @@ public class SnapshotTest extends CloudStackTestNGBase { 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) != Priority.CANT_HANDLE) { - newSnapshot = strategy.takeSnapshot(snapshot); - } + SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.TAKE); + if (snapshotStrategy != null) { + newSnapshot = snapshotStrategy.takeSnapshot(snapshot); } AssertJUnit.assertNotNull(newSnapshot); // create another snapshot for (SnapshotStrategy strategy : this.snapshotStrategies) { - if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { + if (strategy.canHandle(snapshot, SnapshotOperation.DELETE) != Priority.CANT_HANDLE) { strategy.deleteSnapshot(newSnapshot.getId()); } } @@ -451,13 +448,10 @@ public class SnapshotTest extends CloudStackTestNGBase { 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) != Priority.CANT_HANDLE) { - snapshot = strategy.takeSnapshot(snapshot); - result = true; - } + SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.TAKE); + if (snapshotStrategy != null) { + snapshot = snapshotStrategy.takeSnapshot(snapshot); + result = true; } AssertJUnit.assertTrue(result); @@ -484,13 +478,11 @@ public class SnapshotTest extends CloudStackTestNGBase { 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) != Priority.CANT_HANDLE) { - newSnapshot = strategy.takeSnapshot(snapshot); - } + SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.TAKE); + if (snapshotStrategy != null) { + newSnapshot = snapshotStrategy.takeSnapshot(snapshot); } + AssertJUnit.assertNotNull(newSnapshot); LocalHostEndpoint ep = new MockLocalHostEndPoint(); @@ -499,7 +491,7 @@ public class SnapshotTest extends CloudStackTestNGBase { try { for (SnapshotStrategy strategy : this.snapshotStrategies) { - if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { + if (strategy.canHandle(snapshot, SnapshotOperation.DELETE) != Priority.CANT_HANDLE) { boolean res = strategy.deleteSnapshot(newSnapshot.getId()); Assert.assertTrue(res); } 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 3f35e1dd5bc..147f1d4e86d 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 @@ -19,16 +19,18 @@ package org.apache.cloudstack.storage.snapshot; import java.util.Date; +import java.util.List; import javax.inject.Inject; -import org.apache.log4j.Logger; - import org.apache.cloudstack.engine.subsystem.api.storage.DataObjectInStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; 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.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.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.command.CopyCmdAnswer; @@ -37,6 +39,7 @@ import org.apache.cloudstack.storage.datastore.ObjectInDataStoreManager; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.log4j.Logger; import com.cloud.agent.api.Answer; import com.cloud.agent.api.to.DataObjectType; @@ -72,6 +75,8 @@ public class SnapshotObject implements SnapshotInfo { ObjectInDataStoreManager objectInStoreMgr; @Inject SnapshotDataStoreDao snapshotStoreDao; + @Inject + List snapshotStrategies; public SnapshotObject() { @@ -122,6 +127,16 @@ public class SnapshotObject implements SnapshotInfo { return snapshotFactory.getSnapshot(vo.getId(), store); } + @Override + public boolean isRevertable() { + SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.REVERT); + if (snapshotStrategy != null) { + return true; + } + + return false; + } + @Override public VolumeInfo getBaseVolume() { return volFactory.getVolume(snapshot.getVolumeId()); @@ -268,18 +283,18 @@ public class SnapshotObject implements SnapshotInfo { snapshotStore.setParentSnapshotId(0L); } snapshotStoreDao.update(snapshotStore.getId(), snapshotStore); - + // update side-effect of snapshot operation if(snapshotTO.getVolume() != null && snapshotTO.getVolume().getPath() != null) { - VolumeVO vol = volumeDao.findByUuid(snapshotTO.getVolume().getUuid()); - if(vol != null) { - s_logger.info("Update volume path change due to snapshot operation, volume " + vol.getId() + " path: " - + vol.getPath() + "->" + snapshotTO.getVolume().getPath()); - vol.setPath(snapshotTO.getVolume().getPath()); - volumeDao.update(vol.getId(), vol); - } else { - s_logger.error("Cound't find the original volume with uuid: " + snapshotTO.getVolume().getUuid()); - } + VolumeVO vol = volumeDao.findByUuid(snapshotTO.getVolume().getUuid()); + if(vol != null) { + s_logger.info("Update volume path change due to snapshot operation, volume " + vol.getId() + " path: " + + vol.getPath() + "->" + snapshotTO.getVolume().getPath()); + vol.setPath(snapshotTO.getVolume().getPath()); + volumeDao.update(vol.getId(), vol); + } else { + s_logger.error("Cound't find the original volume with uuid: " + snapshotTO.getVolume().getUuid()); + } } } else { throw new CloudRuntimeException("Unknown answer: " + answer.getClass()); 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 6a874d67b8b..b3a64b65a76 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,6 +27,7 @@ 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; @@ -310,7 +311,11 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase { } @Override - public StrategyPriority.Priority canHandle(Snapshot snapshot) { - return StrategyPriority.Priority.DEFAULT; + public StrategyPriority.Priority canHandle(Snapshot snapshot, SnapshotOperation op) { + if (op == SnapshotOperation.REVERT) { + return Priority.CANT_HANDLE; + } + + return Priority.DEFAULT; } } diff --git a/plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java index 3eaeb1f080c..82dc34769b8 100644 --- a/plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java @@ -18,6 +18,35 @@ */ package org.apache.cloudstack.storage.datastore.driver; +import java.util.UUID; + +import javax.inject.Inject; + +import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; +import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; +import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; +import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.framework.async.AsyncCompletionCallback; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.storage.command.CommandResult; +import org.apache.cloudstack.storage.command.CopyCmdAnswer; +import org.apache.cloudstack.storage.command.CopyCommand; +import org.apache.cloudstack.storage.command.CreateObjectCommand; +import org.apache.cloudstack.storage.command.DeleteCommand; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.to.TemplateObjectTO; +import org.apache.cloudstack.storage.volume.VolumeObject; +import org.apache.log4j.Logger; + import com.cloud.agent.api.Answer; import com.cloud.agent.api.storage.ResizeVolumeAnswer; import com.cloud.agent.api.storage.ResizeVolumeCommand; @@ -42,25 +71,6 @@ import com.cloud.template.TemplateManager; import com.cloud.utils.NumbersUtil; import com.cloud.vm.dao.VMInstanceDao; -import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; -import org.apache.cloudstack.engine.subsystem.api.storage.*; -import org.apache.cloudstack.framework.async.AsyncCompletionCallback; -import org.apache.cloudstack.storage.command.CommandResult; -import org.apache.cloudstack.storage.command.CopyCmdAnswer; -import org.apache.cloudstack.storage.command.CopyCommand; -import org.apache.cloudstack.storage.command.CreateObjectCommand; -import org.apache.cloudstack.storage.command.DeleteCommand; -import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.to.TemplateObjectTO; -import org.apache.cloudstack.storage.volume.VolumeObject; - -import org.apache.log4j.Logger; - -import javax.inject.Inject; -import java.util.UUID; - public class CloudStackPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver { private static final Logger s_logger = Logger.getLogger(CloudStackPrimaryDataStoreDriverImpl.class); @Inject @@ -296,5 +306,4 @@ public class CloudStackPrimaryDataStoreDriverImpl implements PrimaryDataStoreDri callback.complete(result); } - } diff --git a/plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java b/plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java index ece7b260cd4..75e8823a1b5 100644 --- a/plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java @@ -16,11 +16,18 @@ // under the License. package org.apache.cloudstack.storage.datastore.driver; -import com.cloud.agent.api.Answer; -import com.cloud.agent.api.to.DataStoreTO; -import com.cloud.agent.api.to.DataTO; -import com.cloud.storage.dao.StoragePoolHostDao; -import org.apache.cloudstack.engine.subsystem.api.storage.*; +import javax.inject.Inject; + +import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; +import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; +import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.framework.async.AsyncRpcContext; @@ -29,7 +36,10 @@ import org.apache.cloudstack.storage.command.CreateObjectCommand; import org.apache.cloudstack.storage.datastore.DataObjectManager; import org.apache.log4j.Logger; -import javax.inject.Inject; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.to.DataStoreTO; +import com.cloud.agent.api.to.DataTO; +import com.cloud.storage.dao.StoragePoolHostDao; public class SamplePrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver { private static final Logger s_logger = Logger.getLogger(SamplePrimaryDataStoreDriverImpl.class); diff --git a/plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java b/plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java index 8046b6cfd1b..a02474d7371 100644 --- a/plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java +++ b/plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java @@ -20,12 +20,19 @@ import java.util.List; import javax.inject.Inject; -import org.apache.cloudstack.engine.subsystem.api.storage.*; +import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; +import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.command.CommandResult; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.util.SolidFireUtil; import org.apache.commons.lang.StringUtils; @@ -39,9 +46,9 @@ import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDetailsDao; -import com.cloud.user.AccountVO; -import com.cloud.user.AccountDetailsDao; import com.cloud.user.AccountDetailVO; +import com.cloud.user.AccountDetailsDao; +import com.cloud.user.AccountVO; import com.cloud.user.dao.AccountDao; public class SolidfirePrimaryDataStoreDriver implements PrimaryDataStoreDriver { @@ -122,10 +129,10 @@ public class SolidfirePrimaryDataStoreDriver implements PrimaryDataStoreDriver { String clusterAdminPassword = sfConnection.getClusterAdminPassword(); long accountNumber = SolidFireUtil.createSolidFireAccount(mVip, mPort, - clusterAdminUsername, clusterAdminPassword, sfAccountName); + clusterAdminUsername, clusterAdminPassword, sfAccountName); return SolidFireUtil.getSolidFireAccountById(mVip, mPort, - clusterAdminUsername, clusterAdminPassword, accountNumber); + clusterAdminUsername, clusterAdminPassword, accountNumber); } private void updateCsDbWithAccountInfo(long csAccountId, SolidFireUtil.SolidFireAccount sfAccount) { @@ -174,18 +181,22 @@ public class SolidfirePrimaryDataStoreDriver implements PrimaryDataStoreDriver { _targetSecret = targetSecret; } + @Override public String getInitiatorUsername() { return _initiatorUsername; } + @Override public String getInitiatorSecret() { return _initiatorSecret; } + @Override public String getTargetUsername() { return _targetUsername; } + @Override public String getTargetSecret() { return _targetSecret; } @@ -268,7 +279,7 @@ public class SolidfirePrimaryDataStoreDriver implements PrimaryDataStoreDriver { Long maxIops = volumeInfo.getMaxIops(); if (minIops == null || minIops <= 0 || - maxIops == null || maxIops <= 0) { + maxIops == null || maxIops <= 0) { long defaultMaxIops = getDefaultMaxIops(storagePoolId); iops = new Iops(getDefaultMinIops(storagePoolId), defaultMaxIops, getDefaultBurstIops(storagePoolId, defaultMaxIops)); @@ -328,22 +339,22 @@ public class SolidfirePrimaryDataStoreDriver implements PrimaryDataStoreDriver { _minIops = minIops; _maxIops = maxIops; _burstIops = burstIops; - } + } - public long getMinIops() - { - return _minIops; - } + public long getMinIops() + { + return _minIops; + } - public long getMaxIops() - { - return _maxIops; - } + public long getMaxIops() + { + return _maxIops; + } - public long getBurstIops() - { - return _burstIops; - } + public long getBurstIops() + { + return _burstIops; + } } private void deleteSolidFireVolume(VolumeInfo volumeInfo, SolidFireConnection sfConnection) @@ -501,14 +512,14 @@ public class SolidfirePrimaryDataStoreDriver implements PrimaryDataStoreDriver { _volumeDao.deleteVolumesByInstance(volumeInfo.getId()); -// if (!sfAccountHasVolume(sfAccountId, sfConnection)) { -// // delete the account from the SolidFire SAN -// deleteSolidFireAccount(sfAccountId, sfConnection); -// -// // delete the info in the account_details table -// // that's related to the SolidFire account -// _accountDetailsDao.deleteDetails(account.getAccountId()); -// } + // if (!sfAccountHasVolume(sfAccountId, sfConnection)) { + // // delete the account from the SolidFire SAN + // deleteSolidFireAccount(sfAccountId, sfConnection); + // + // // delete the info in the account_details table + // // that's related to the SolidFire account + // _accountDetailsDao.deleteDetails(account.getAccountId()); + // } StoragePoolVO storagePool = _storagePoolDao.findById(storagePoolId); diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java index 6978c9be850..77be43f2249 100755 --- a/server/src/com/cloud/api/ApiResponseHelper.java +++ b/server/src/com/cloud/api/ApiResponseHelper.java @@ -31,8 +31,6 @@ import java.util.TimeZone; import javax.inject.Inject; -import org.apache.log4j.Logger; - import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.affinity.AffinityGroup; @@ -134,6 +132,8 @@ import org.apache.cloudstack.api.response.VpnUsersResponse; import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.cloudstack.config.Configuration; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.framework.jobs.AsyncJob; import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.cloudstack.network.lb.ApplicationLoadBalancerRule; @@ -144,6 +144,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.usage.Usage; import org.apache.cloudstack.usage.UsageService; import org.apache.cloudstack.usage.UsageTypes; +import org.apache.log4j.Logger; import com.cloud.api.query.ViewResponseHelper; import com.cloud.api.query.vo.AccountJoinVO; @@ -257,6 +258,7 @@ import com.cloud.server.Criteria; import com.cloud.server.ResourceTag; import com.cloud.server.ResourceTag.TaggedResourceType; import com.cloud.service.ServiceOfferingVO; +import com.cloud.storage.DataStoreRole; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.GuestOS; import com.cloud.storage.GuestOSCategoryVO; @@ -281,6 +283,7 @@ import com.cloud.uservm.UserVm; import com.cloud.utils.Pair; import com.cloud.utils.StringUtils; import com.cloud.utils.db.EntityManager; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.Ip; import com.cloud.utils.net.NetUtils; import com.cloud.vm.ConsoleProxyVO; @@ -312,6 +315,8 @@ public class ApiResponseHelper implements ResponseGenerator { protected AsyncJobManager _jobMgr; @Inject ConfigurationManager _configMgr; + @Inject + SnapshotDataFactory snapshotfactory; @Override public UserResponse createUserResponse(User user) { @@ -447,6 +452,19 @@ public class ApiResponseHelper implements ResponseGenerator { snapshotResponse.setIntervalType(ApiDBUtils.getSnapshotIntervalTypes(snapshot.getId())); snapshotResponse.setState(snapshot.getState()); + SnapshotInfo snapshotInfo = null; + if (!(snapshot instanceof SnapshotInfo)) { + snapshotInfo = snapshotfactory.getSnapshot(snapshot.getId(), DataStoreRole.Image); + } else { + snapshotInfo = (SnapshotInfo)snapshot; + } + + if (snapshotInfo == null) { + throw new CloudRuntimeException("Unable to find info for image store snapshot with uuid '"+snapshot.getUuid()+"'"); + } + + snapshotResponse.setRevertable(snapshotInfo.isRevertable()); + // set tag information List tags = ApiDBUtils.listByResourceTypeAndId(TaggedResourceType.Snapshot, snapshot.getId()); List tagResponses = new ArrayList(); diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java index dade9837d90..7073c64a950 100755 --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -40,8 +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.SnapshotStrategy.SnapshotOperation; 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; @@ -278,17 +278,10 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager, } } - StrategyPriority.sortStrategies(snapshotStrategies, snapshot); - - SnapshotStrategy snapshotStrategy = null; - for (SnapshotStrategy strategy : snapshotStrategies) { - if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { - snapshotStrategy = strategy; - break; - } - } + SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.REVERT); if (snapshotStrategy == null) { + s_logger.error("Unable to find snaphot strategy to handle snapshot with id '"+snapshotId+"'"); return false; } @@ -517,16 +510,13 @@ 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) != Priority.CANT_HANDLE) { - snapshotStrategy = strategy; - break; - } + SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshotCheck, SnapshotOperation.DELETE); + if (snapshotStrategy == null) { + s_logger.error("Unable to find snaphot strategy to handle snapshot with id '"+snapshotId+"'"); + return false; } + try { boolean result = snapshotStrategy.deleteSnapshot(snapshotId); if (result) { @@ -711,17 +701,12 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager, // Either way delete the snapshots for this volume. List snapshots = listSnapsforVolume(volumeId); 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) != Priority.CANT_HANDLE) { - snapshotStrategy = strategy; - break; - } + SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.DELETE); + if (snapshotStrategy == null) { + s_logger.error("Unable to find snaphot strategy to handle snapshot with id '"+snapshot.getId()+"'"); + continue; } + if (snapshotStrategy.deleteSnapshot(snapshot.getId())) { if (snapshot.getRecurringType() == Type.MANUAL) { _resourceLimitMgr.decrementResourceCount(accountId, ResourceType.snapshot); @@ -1045,22 +1030,16 @@ public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager, Long snapshotId = payload.getSnapshotId(); Account snapshotOwner = payload.getAccount(); SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId, volume.getDataStore()); - boolean processed = false; - - StrategyPriority.sortStrategies(snapshotStrategies, snapshot); try { - for (SnapshotStrategy strategy : snapshotStrategies) { - if (strategy.canHandle(snapshot) != Priority.CANT_HANDLE) { - processed = true; - snapshot = strategy.takeSnapshot(snapshot); - break; - } - } - if (!processed) { + SnapshotStrategy snapshotStrategy = StrategyPriority.pickStrategy(snapshotStrategies, snapshot, SnapshotOperation.TAKE); + + if (snapshotStrategy == null) { throw new CloudRuntimeException("Can't find snapshot strategy to deal with snapshot:" + snapshotId); } + snapshotStrategy.takeSnapshot(snapshot); + try { postCreateSnapshot(volume.getId(), snapshotId, payload.getSnapshotPolicyId()); diff --git a/ui/scripts/storage.js b/ui/scripts/storage.js index b16f4d4e77e..314621e3368 100644 --- a/ui/scripts/storage.js +++ b/ui/scripts/storage.js @@ -1960,7 +1960,7 @@ allowedActions.push("createTemplate"); allowedActions.push("createVolume"); - if (args.context.volumes[0].vmstate == "Stopped") { + if (jsonObj.revertable && args.context.volumes[0].vmstate == "Stopped") { allowedActions.push("revertSnapshot"); } }