Skip to content

Commit 0ed7ebd

Browse files
csuich2sudison
csuich2
authored and
sudison
committed
Squashed & merged commit of the following:
commit c9ee0d12e191e803fb341f3f96e95ca434a36f6c Author: Wei Zhou <w.zhou@leaseweb.com> Date: Wed Oct 23 16:55:10 2013 +0200 CLOUDSTACK-4931, CLOUDSTACK-4937: setDetails to user VMs only (cherry picked from commit a94acc5) commit fe1586c71377bc6d219db2dcf088c40b65dd1fc4 Author: Anthony Xu <anthony.xu@citrix.com> 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 <anthony.xu@citrix.com> 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 <laszlo.hornyak@gmail.com> Date: Mon Oct 21 21:17:33 2013 +0200 removed unused static field - s_httpClientManager was not used Signed-off-by: Laszlo Hornyak <laszlo.hornyak@gmail.com> commit d4121fa26023db236f7396cea455ef090672ae9a Author: Chris Suich <chris.suich@netapp.com> Date: Tue Oct 22 10:45:22 2013 -0400 Updated DataMotionServiceImpl and ApiResponseHelper based on review feedback. commit aaf026e1e4204d405bcda2ae4f1a01b1d0f7e7cb Author: Chris Suich <chris.suich@netapp.com> 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 <chris.suich@netapp.com> 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 <chris.suich@netapp.com> 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 <sudison@gmail.com>
1 parent 4b530c8 commit 0ed7ebd

18 files changed

Lines changed: 441 additions & 357 deletions

File tree

‎api/src/org/apache/cloudstack/api/ApiConstants.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public class ApiConstants {
3434
public static final String BYTES_READ_RATE = "bytesreadrate";
3535
public static final String BYTES_WRITE_RATE = "byteswriterate";
3636
public static final String CATEGORY = "category";
37+
public static final String CAN_REVERT = "canrevert";
3738
public static final String CERTIFICATE = "certificate";
3839
public static final String PRIVATE_KEY = "privatekey";
3940
public static final String DOMAIN_SUFFIX = "domainsuffix";
@@ -187,6 +188,7 @@ public class ApiConstants {
187188
public static final String REQUIRES_HVM = "requireshvm";
188189
public static final String RESOURCE_TYPE = "resourcetype";
189190
public static final String RESPONSE = "response";
191+
public static final String REVERTABLE = "revertable";
190192
public static final String QUERY_FILTER = "queryfilter";
191193
public static final String SCHEDULE = "schedule";
192194
public static final String SCOPE = "scope";
@@ -249,7 +251,7 @@ public class ApiConstants {
249251
public static final String IS_VOLATILE = "isvolatile";
250252
public static final String VOLUME_ID = "volumeid";
251253
public static final String ZONE_ID = "zoneid";
252-
public static final String ZONE_NAME = "zonename";
254+
public static final String ZONE_NAME = "zonename";
253255
public static final String NETWORK_TYPE = "networktype";
254256
public static final String PAGE = "page";
255257
public static final String PAGE_SIZE = "pagesize";

‎api/src/org/apache/cloudstack/api/response/SnapshotResponse.java‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,8 @@
2626
import com.cloud.serializer.Param;
2727
import com.cloud.storage.Snapshot;
2828
import com.google.gson.annotations.SerializedName;
29-
import com.cloud.serializer.Param;
30-
import com.cloud.storage.Snapshot;
31-
import com.google.gson.annotations.SerializedName;
32-
import org.apache.cloudstack.api.ApiConstants;
33-
import org.apache.cloudstack.api.BaseResponse;
34-
import org.apache.cloudstack.api.EntityReference;
35-
36-
import java.util.Date;
37-
import java.util.List;
3829

3930
@EntityReference(value=Snapshot.class)
40-
@SuppressWarnings("unused")
4131
public class SnapshotResponse extends BaseResponse implements ControlledEntityResponse {
4232
@SerializedName(ApiConstants.ID)
4333
@Param(description = "ID of the snapshot")
@@ -100,6 +90,9 @@ public class SnapshotResponse extends BaseResponse implements ControlledEntityRe
10090
@SerializedName(ApiConstants.TAGS) @Param(description="the list of resource tags associated with snapshot", responseObject = ResourceTagResponse.class)
10191
private List<ResourceTagResponse> tags;
10292

93+
@SerializedName(ApiConstants.REVERTABLE)
94+
@Param(description="indicates whether the underlying storage supports reverting the volume to this snapshot")
95+
private boolean revertable;
10396

10497
@Override
10598
public String getObjectId() {
@@ -118,6 +111,7 @@ public String getAccountName() {
118111
return accountName;
119112
}
120113

114+
@Override
121115
public void setAccountName(String accountName) {
122116
this.accountName = accountName;
123117
}
@@ -131,6 +125,7 @@ public void setDomainId(String domainId) {
131125
this.domainId = domainId;
132126
}
133127

128+
@Override
134129
public void setDomainName(String domainName) {
135130
this.domainName = domainName;
136131
}
@@ -180,8 +175,16 @@ public void setProjectName(String projectName) {
180175
public void setZoneId(String zoneId) {
181176
this.zoneId = zoneId;
182177
}
183-
178+
184179
public void setTags(List<ResourceTagResponse> tags) {
185180
this.tags = tags;
186181
}
182+
183+
public boolean isRevertable() {
184+
return revertable;
185+
}
186+
187+
public void setRevertable(boolean revertable) {
188+
this.revertable = revertable;
189+
}
187190
}

‎engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,6 @@ public interface SnapshotInfo extends DataObject, Snapshot {
3232
Long getDataCenterId();
3333

3434
ObjectInDataStoreStateMachine.State getStatus();
35+
36+
boolean isRevertable();
3537
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@
1919
import com.cloud.storage.Snapshot;
2020

2121
public interface SnapshotStrategy {
22+
enum SnapshotOperation {
23+
TAKE,
24+
BACKUP,
25+
DELETE,
26+
REVERT
27+
}
28+
2229
SnapshotInfo takeSnapshot(SnapshotInfo snapshot);
2330

2431
SnapshotInfo backupSnapshot(SnapshotInfo snapshot);
@@ -27,5 +34,5 @@ public interface SnapshotStrategy {
2734

2835
boolean revertSnapshot(Long snapshotId);
2936

30-
StrategyPriority.Priority canHandle(Snapshot snapshot);
37+
StrategyPriority.Priority canHandle(Snapshot snapshot, SnapshotOperation op);
3138
}

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

Lines changed: 34 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@
1616
// under the License.
1717
package org.apache.cloudstack.engine.subsystem.api.storage;
1818

19-
import java.util.Collections;
20-
import java.util.Comparator;
2119
import java.util.List;
2220
import java.util.Map;
2321

22+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
23+
2424
import com.cloud.host.Host;
2525
import com.cloud.storage.Snapshot;
2626

@@ -33,67 +33,49 @@ public enum Priority {
3333
HIGHEST
3434
}
3535

36-
public static void sortStrategies(List<SnapshotStrategy> strategies, Snapshot snapshot) {
37-
Collections.sort(strategies, new SnapshotStrategyComparator(snapshot));
38-
}
39-
40-
public static void sortStrategies(List<DataMotionStrategy> strategies, DataObject srcData, DataObject destData) {
41-
Collections.sort(strategies, new DataMotionStrategyComparator(srcData, destData));
42-
}
43-
44-
public static void sortStrategies(List<DataMotionStrategy> strategies, Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
45-
Collections.sort(strategies, new DataMotionStrategyHostComparator(volumeMap, srcHost, destHost));
46-
}
47-
48-
static class SnapshotStrategyComparator implements Comparator<SnapshotStrategy> {
36+
public static SnapshotStrategy pickStrategy(List<SnapshotStrategy> strategies, Snapshot snapshot, SnapshotOperation op) {
37+
Priority highestPriority = Priority.CANT_HANDLE;
38+
SnapshotStrategy strategyToUse = null;
4939

50-
Snapshot snapshot;
51-
52-
public SnapshotStrategyComparator(Snapshot snapshot) {
53-
this.snapshot = snapshot;
40+
for (SnapshotStrategy strategy : strategies) {
41+
Priority priority = strategy.canHandle(snapshot, op);
42+
if (priority.ordinal() > highestPriority.ordinal()) {
43+
highestPriority = priority;
44+
strategyToUse = strategy;
45+
}
5446
}
5547

56-
@Override
57-
public int compare(SnapshotStrategy o1, SnapshotStrategy o2) {
58-
int i1 = o1.canHandle(snapshot).ordinal();
59-
int i2 = o2.canHandle(snapshot).ordinal();
60-
return Integer.compare(i2, i1);
61-
}
48+
return strategyToUse;
6249
}
6350

64-
static class DataMotionStrategyComparator implements Comparator<DataMotionStrategy> {
65-
66-
DataObject srcData, destData;
67-
68-
public DataMotionStrategyComparator(DataObject srcData, DataObject destData) {
69-
this.srcData = srcData;
70-
this.destData = destData;
51+
// TODO DRY this out by consolidating methods
52+
public static DataMotionStrategy pickStrategy(List<DataMotionStrategy> strategies, DataObject srcData, DataObject destData) {
53+
Priority highestPriority = Priority.CANT_HANDLE;
54+
DataMotionStrategy strategyToUse = null;
55+
56+
for (DataMotionStrategy strategy : strategies) {
57+
Priority priority = strategy.canHandle(srcData, destData);
58+
if (priority.ordinal() > highestPriority.ordinal()) {
59+
highestPriority = priority;
60+
strategyToUse = strategy;
61+
}
7162
}
7263

73-
@Override
74-
public int compare(DataMotionStrategy o1, DataMotionStrategy o2) {
75-
int i1 = o1.canHandle(srcData, destData).ordinal();
76-
int i2 = o2.canHandle(srcData, destData).ordinal();
77-
return Integer.compare(i2, i1);
78-
}
64+
return strategyToUse;
7965
}
8066

81-
static class DataMotionStrategyHostComparator implements Comparator<DataMotionStrategy> {
82-
83-
Host srcHost, destHost;
84-
Map<VolumeInfo, DataStore> volumeMap;
67+
public static DataMotionStrategy pickStrategy(List<DataMotionStrategy> strategies, Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
68+
Priority highestPriority = Priority.CANT_HANDLE;
69+
DataMotionStrategy strategyToUse = null;
8570

86-
public DataMotionStrategyHostComparator(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
87-
this.volumeMap = volumeMap;
88-
this.srcHost = srcHost;
89-
this.destHost = destHost;
71+
for (DataMotionStrategy strategy : strategies) {
72+
Priority priority = strategy.canHandle(volumeMap, srcHost, destHost);
73+
if (priority.ordinal() > highestPriority.ordinal()) {
74+
highestPriority = priority;
75+
strategyToUse = strategy;
76+
}
9077
}
9178

92-
@Override
93-
public int compare(DataMotionStrategy o1, DataMotionStrategy o2) {
94-
int i1 = o1.canHandle(volumeMap, srcHost, destHost).ordinal();
95-
int i2 = o2.canHandle(volumeMap, srcHost, destHost).ordinal();
96-
return Integer.compare(i2, i1);
97-
}
79+
return strategyToUse;
9880
}
9981
}

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

Lines changed: 63 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
package org.apache.cloudstack.engine.subsystem.api.storage;
1818

1919
import java.util.ArrayList;
20-
import java.util.Arrays;
2120
import java.util.List;
2221
import java.util.Map;
2322

23+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
2424
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority.Priority;
2525
import org.junit.Test;
2626

@@ -43,22 +43,34 @@ public void testSortSnapshotStrategies() {
4343
SnapshotStrategy pluginStrategy = mock(SnapshotStrategy.class);
4444
SnapshotStrategy highestStrategy = mock(SnapshotStrategy.class);
4545

46-
doReturn(Priority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(Snapshot.class));
47-
doReturn(Priority.DEFAULT).when(defaultStrategy).canHandle(any(Snapshot.class));
48-
doReturn(Priority.HYPERVISOR).when(hyperStrategy).canHandle(any(Snapshot.class));
49-
doReturn(Priority.PLUGIN).when(pluginStrategy).canHandle(any(Snapshot.class));
50-
doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(Snapshot.class));
46+
doReturn(Priority.CANT_HANDLE).when(cantHandleStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
47+
doReturn(Priority.DEFAULT).when(defaultStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
48+
doReturn(Priority.HYPERVISOR).when(hyperStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
49+
doReturn(Priority.PLUGIN).when(pluginStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
50+
doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(Snapshot.class), any(SnapshotOperation.class));
5151

5252
List<SnapshotStrategy> strategies = new ArrayList<SnapshotStrategy>(5);
53-
strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy));
53+
SnapshotStrategy strategy = null;
5454

55-
StrategyPriority.sortStrategies(strategies, mock(Snapshot.class));
55+
strategies.add(cantHandleStrategy);
56+
strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
57+
assertEquals("A strategy was found when it shouldn't have been.", null, strategy);
5658

57-
assertEquals("Highest was not 1st.", highestStrategy, strategies.get(0));
58-
assertEquals("Plugin was not 2nd.", pluginStrategy, strategies.get(1));
59-
assertEquals("Hypervisor was not 3rd.", hyperStrategy, strategies.get(2));
60-
assertEquals("Default was not 4th.", defaultStrategy, strategies.get(3));
61-
assertEquals("Can't Handle was not 5th.", cantHandleStrategy, strategies.get(4));
59+
strategies.add(defaultStrategy);
60+
strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
61+
assertEquals("Default strategy was not picked.", defaultStrategy, strategy);
62+
63+
strategies.add(hyperStrategy);
64+
strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
65+
assertEquals("Hypervisor strategy was not picked.", hyperStrategy, strategy);
66+
67+
strategies.add(pluginStrategy);
68+
strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
69+
assertEquals("Plugin strategy was not picked.", pluginStrategy, strategy);
70+
71+
strategies.add(highestStrategy);
72+
strategy = StrategyPriority.pickStrategy(strategies, mock(Snapshot.class), SnapshotOperation.TAKE);
73+
assertEquals("Highest strategy was not picked.", highestStrategy, strategy);
6274
}
6375

6476
@Test
@@ -76,15 +88,27 @@ public void testSortDataMotionStrategies() {
7688
doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(DataObject.class), any(DataObject.class));
7789

7890
List<DataMotionStrategy> strategies = new ArrayList<DataMotionStrategy>(5);
79-
strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy));
91+
DataMotionStrategy strategy = null;
92+
93+
strategies.add(cantHandleStrategy);
94+
strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
95+
assertEquals("A strategy was found when it shouldn't have been.", null, strategy);
96+
97+
strategies.add(defaultStrategy);
98+
strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
99+
assertEquals("Default strategy was not picked.", defaultStrategy, strategy);
80100

81-
StrategyPriority.sortStrategies(strategies, mock(DataObject.class), mock(DataObject.class));
101+
strategies.add(hyperStrategy);
102+
strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
103+
assertEquals("Hypervisor strategy was not picked.", hyperStrategy, strategy);
82104

83-
assertEquals("Highest was not 1st.", highestStrategy, strategies.get(0));
84-
assertEquals("Plugin was not 2nd.", pluginStrategy, strategies.get(1));
85-
assertEquals("Hypervisor was not 3rd.", hyperStrategy, strategies.get(2));
86-
assertEquals("Default was not 4th.", defaultStrategy, strategies.get(3));
87-
assertEquals("Can't Handle was not 5th.", cantHandleStrategy, strategies.get(4));
105+
strategies.add(pluginStrategy);
106+
strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
107+
assertEquals("Plugin strategy was not picked.", pluginStrategy, strategy);
108+
109+
strategies.add(highestStrategy);
110+
strategy = StrategyPriority.pickStrategy(strategies, mock(DataObject.class), mock(DataObject.class));
111+
assertEquals("Highest strategy was not picked.", highestStrategy, strategy);
88112
}
89113

90114
@Test
@@ -103,14 +127,26 @@ public void testSortDataMotionStrategies2() {
103127
doReturn(Priority.HIGHEST).when(highestStrategy).canHandle(any(Map.class), any(Host.class), any(Host.class));
104128

105129
List<DataMotionStrategy> strategies = new ArrayList<DataMotionStrategy>(5);
106-
strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy, highestStrategy));
130+
DataMotionStrategy strategy = null;
131+
132+
strategies.add(cantHandleStrategy);
133+
strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
134+
assertEquals("A strategy was found when it shouldn't have been.", null, strategy);
135+
136+
strategies.add(defaultStrategy);
137+
strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
138+
assertEquals("Default strategy was not picked.", defaultStrategy, strategy);
139+
140+
strategies.add(hyperStrategy);
141+
strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
142+
assertEquals("Hypervisor strategy was not picked.", hyperStrategy, strategy);
107143

108-
StrategyPriority.sortStrategies(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
144+
strategies.add(pluginStrategy);
145+
strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
146+
assertEquals("Plugin strategy was not picked.", pluginStrategy, strategy);
109147

110-
assertEquals("Highest was not 1st.", highestStrategy, strategies.get(0));
111-
assertEquals("Plugin was not 2nd.", pluginStrategy, strategies.get(1));
112-
assertEquals("Hypervisor was not 3rd.", hyperStrategy, strategies.get(2));
113-
assertEquals("Default was not 4th.", defaultStrategy, strategies.get(3));
114-
assertEquals("Can't Handle was not 5th.", cantHandleStrategy, strategies.get(4));
148+
strategies.add(highestStrategy);
149+
strategy = StrategyPriority.pickStrategy(strategies, mock(Map.class), mock(Host.class), mock(Host.class));
150+
assertEquals("Highest strategy was not picked.", highestStrategy, strategy);
115151
}
116152
}

0 commit comments

Comments
 (0)