Skip to content

Commit d549f3b

Browse files
radu-todiricaradu-todirica
authored
Add cache mode param properly (#3925)
1 parent 2d63ed5 commit d549f3b

12 files changed

Lines changed: 311 additions & 11 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ public class ApiConstants {
781781
public static final String ROUTER_CHECK_TYPE = "checktype";
782782
public static final String LAST_UPDATED = "lastupdated";
783783
public static final String PERFORM_FRESH_CHECKS = "performfreshchecks";
784+
public static final String CACHE_MODE = "cachemode";
784785

785786
public static final String CONSOLE_END_POINT = "consoleendpoint";
786787
public static final String EXTERNAL_LOAD_BALANCER_IP_ADDRESS = "externalloadbalanceripaddress";

‎api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,13 @@ public class CreateDiskOfferingCmd extends BaseCmd {
144144
description = "Hypervisor snapshot reserve space as a percent of a volume (for managed storage using Xen or VMware)")
145145
private Integer hypervisorSnapshotReserve;
146146

147+
@Parameter(name = ApiConstants.CACHE_MODE,
148+
type = CommandType.STRING,
149+
required = false,
150+
description = "the cache mode to use for this disk offering. none, writeback or writethrough",
151+
since = "4.14")
152+
private String cacheMode;
153+
147154
/////////////////////////////////////////////////////
148155
/////////////////// Accessors ///////////////////////
149156
/////////////////////////////////////////////////////
@@ -262,6 +269,10 @@ public Integer getHypervisorSnapshotReserve() {
262269
return hypervisorSnapshotReserve;
263270
}
264271

272+
public String getCacheMode() {
273+
return cacheMode;
274+
}
275+
265276
/////////////////////////////////////////////////////
266277
/////////////// API Implementation///////////////////
267278
/////////////////////////////////////////////////////

‎api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,13 @@ public class CreateServiceOfferingCmd extends BaseCmd {
178178
since = "4.4")
179179
private Integer hypervisorSnapshotReserve;
180180

181+
@Parameter(name = ApiConstants.CACHE_MODE,
182+
type = CommandType.STRING,
183+
required = false,
184+
description = "the cache mode to use for this disk offering. none, writeback or writethrough",
185+
since = "4.14")
186+
private String cacheMode;
187+
181188
// Introduce 4 new optional paramaters to work custom compute offerings
182189
@Parameter(name = ApiConstants.CUSTOMIZED,
183190
type = CommandType.BOOLEAN,
@@ -377,6 +384,10 @@ public Integer getHypervisorSnapshotReserve() {
377384
return hypervisorSnapshotReserve;
378385
}
379386

387+
public String getCacheMode() {
388+
return cacheMode;
389+
}
390+
380391
/**
381392
* If customized parameter is true, then cpuNumber, memory and cpuSpeed must be null
382393
* Check if the optional params min/max CPU/Memory have been specified

‎api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,10 @@ public class ServiceOfferingResponse extends BaseResponse {
192192
@Param(description = "is true if the offering is customized", since = "4.3.0")
193193
private Boolean isCustomized;
194194

195+
@SerializedName("cacheMode")
196+
@Param(description = "the cache mode to use for this disk offering. none, writeback or writethrough", since = "4.14")
197+
private String cacheMode;
198+
195199
public ServiceOfferingResponse() {
196200
}
197201

@@ -448,4 +452,7 @@ public void setIscutomized(boolean iscutomized) {
448452

449453
}
450454

455+
public void setCacheMode(String cacheMode) {
456+
this.cacheMode = cacheMode;
457+
}
451458
}

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,7 +1171,7 @@ protected synchronized String attachOrDetachDisk(final Connect conn, final boole
11711171
final Long bytesReadRate, final Long bytesReadRateMax, final Long bytesReadRateMaxLength,
11721172
final Long bytesWriteRate, final Long bytesWriteRateMax, final Long bytesWriteRateMaxLength,
11731173
final Long iopsReadRate, final Long iopsReadRateMax, final Long iopsReadRateMaxLength,
1174-
final Long iopsWriteRate, final Long iopsWriteRateMax, final Long iopsWriteRateMaxLength) throws LibvirtException, InternalErrorException {
1174+
final Long iopsWriteRate, final Long iopsWriteRateMax, final Long iopsWriteRateMaxLength, final String cacheMode) throws LibvirtException, InternalErrorException {
11751175
List<DiskDef> disks = null;
11761176
Domain dm = null;
11771177
DiskDef diskdef = null;
@@ -1281,6 +1281,9 @@ protected synchronized String attachOrDetachDisk(final Connect conn, final boole
12811281
if ((iopsWriteRateMaxLength != null) && (iopsWriteRateMaxLength > 0)) {
12821282
diskdef.setIopsWriteRateMaxLength(iopsWriteRateMaxLength);
12831283
}
1284+
if(cacheMode != null) {
1285+
diskdef.setCacheMode(DiskDef.DiskCacheMode.valueOf(cacheMode.toUpperCase()));
1286+
}
12841287
}
12851288

12861289
final String xml = diskdef.toString();
@@ -1305,12 +1308,13 @@ public Answer attachVolume(final AttachCommand cmd) {
13051308
storagePoolMgr.connectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), vol.getPath(), disk.getDetails());
13061309

13071310
final KVMPhysicalDisk phyDisk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), vol.getPath());
1311+
final String volCacheMode = vol.getCacheMode() == null ? null : vol.getCacheMode().toString();
13081312

13091313
attachOrDetachDisk(conn, true, vmName, phyDisk, disk.getDiskSeq().intValue(), serial,
13101314
vol.getBytesReadRate(), vol.getBytesReadRateMax(), vol.getBytesReadRateMaxLength(),
13111315
vol.getBytesWriteRate(), vol.getBytesWriteRateMax(), vol.getBytesWriteRateMaxLength(),
13121316
vol.getIopsReadRate(), vol.getIopsReadRateMax(), vol.getIopsReadRateMaxLength(),
1313-
vol.getIopsWriteRate(), vol.getIopsWriteRateMax(), vol.getIopsWriteRateMaxLength());
1317+
vol.getIopsWriteRate(), vol.getIopsWriteRateMax(), vol.getIopsWriteRateMaxLength(), volCacheMode);
13141318

13151319
return new AttachAnswer(disk);
13161320
} catch (final LibvirtException e) {
@@ -1334,12 +1338,13 @@ public Answer dettachVolume(final DettachCommand cmd) {
13341338
final Connect conn = LibvirtConnection.getConnectionByVmName(vmName);
13351339

13361340
final KVMPhysicalDisk phyDisk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), vol.getPath());
1341+
final String volCacheMode = vol.getCacheMode() == null ? null : vol.getCacheMode().toString();
13371342

13381343
attachOrDetachDisk(conn, false, vmName, phyDisk, disk.getDiskSeq().intValue(), serial,
13391344
vol.getBytesReadRate(), vol.getBytesReadRateMax(), vol.getBytesReadRateMaxLength(),
13401345
vol.getBytesWriteRate(), vol.getBytesWriteRateMax(), vol.getBytesWriteRateMaxLength(),
13411346
vol.getIopsReadRate(), vol.getIopsReadRateMax(), vol.getIopsReadRateMaxLength(),
1342-
vol.getIopsWriteRate(), vol.getIopsWriteRateMax(), vol.getIopsWriteRateMaxLength());
1347+
vol.getIopsWriteRate(), vol.getIopsWriteRateMax(), vol.getIopsWriteRateMaxLength(), volCacheMode);
13431348

13441349
storagePoolMgr.disconnectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), vol.getPath());
13451350

‎server/src/main/java/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ public ServiceOfferingResponse newServiceOfferingResponse(ServiceOfferingJoinVO
102102
offeringResponse.setDetails(ApiDBUtils.getResourceDetails(offering.getId(), ResourceObjectType.ServiceOffering));
103103
offeringResponse.setObjectName("serviceoffering");
104104
offeringResponse.setIscutomized(offering.isDynamic());
105+
offeringResponse.setCacheMode(offering.getCacheMode());
105106

106107
return offeringResponse;
107108
}

‎server/src/main/java/com/cloud/api/query/vo/ServiceOfferingJoinVO.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,9 @@ public class ServiceOfferingJoinVO extends BaseViewVO implements InternalIdentit
172172
@Column(name = "deployment_planner")
173173
private String deploymentPlanner;
174174

175+
@Column(name = "cache_mode")
176+
String cacheMode;
177+
175178
public ServiceOfferingJoinVO() {
176179
}
177180

@@ -349,4 +352,8 @@ public Long getIopsWriteRate() {
349352
public boolean isDynamic() {
350353
return cpu == null || speed == null || ramSize == null;
351354
}
355+
356+
public String getCacheMode() {
357+
return cacheMode;
358+
}
352359
}

‎server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@
245245
import com.cloud.vm.dao.NicIpAliasVO;
246246
import com.cloud.vm.dao.NicSecondaryIpDao;
247247
import com.cloud.vm.dao.VMInstanceDao;
248+
import com.google.common.base.Enums;
248249
import com.google.common.base.MoreObjects;
249250
import com.google.common.base.Preconditions;
250251
import com.google.common.base.Strings;
@@ -2353,6 +2354,9 @@ public ServiceOffering createServiceOffering(final CreateServiceOfferingCmd cmd)
23532354
}
23542355
}
23552356

2357+
// check if cache_mode parameter is valid
2358+
validateCacheMode(cmd.getCacheMode());
2359+
23562360
final Boolean offerHA = cmd.isOfferHa();
23572361

23582362
boolean localStorageRequired = false;
@@ -2433,7 +2437,7 @@ public ServiceOffering createServiceOffering(final CreateServiceOfferingCmd cmd)
24332437
cmd.getBytesWriteRate(), cmd.getBytesWriteRateMax(), cmd.getBytesWriteRateMaxLength(),
24342438
cmd.getIopsReadRate(), cmd.getIopsReadRateMax(), cmd.getIopsReadRateMaxLength(),
24352439
cmd.getIopsWriteRate(), cmd.getIopsWriteRateMax(), cmd.getIopsWriteRateMaxLength(),
2436-
cmd.getHypervisorSnapshotReserve());
2440+
cmd.getHypervisorSnapshotReserve(), cmd.getCacheMode());
24372441
}
24382442

24392443
protected ServiceOfferingVO createServiceOffering(final long userId, final boolean isSystem, final VirtualMachine.Type vmType,
@@ -2444,7 +2448,7 @@ protected ServiceOfferingVO createServiceOffering(final long userId, final boole
24442448
Long bytesWriteRate, Long bytesWriteRateMax, Long bytesWriteRateMaxLength,
24452449
Long iopsReadRate, Long iopsReadRateMax, Long iopsReadRateMaxLength,
24462450
Long iopsWriteRate, Long iopsWriteRateMax, Long iopsWriteRateMaxLength,
2447-
final Integer hypervisorSnapshotReserve) {
2451+
final Integer hypervisorSnapshotReserve, String cacheMode) {
24482452
// Filter child domains when both parent and child domains are present
24492453
List<Long> filteredDomainIds = filterChildSubDomains(domainIds);
24502454

@@ -2541,6 +2545,9 @@ protected ServiceOfferingVO createServiceOffering(final long userId, final boole
25412545
if (iopsWriteRateMaxLength != null && iopsWriteRateMaxLength > 0) {
25422546
offering.setIopsWriteRateMaxLength(iopsWriteRateMaxLength);
25432547
}
2548+
if(cacheMode != null) {
2549+
offering.setCacheMode(DiskOffering.DiskCacheMode.valueOf(cacheMode.toUpperCase()));
2550+
}
25442551

25452552
if (hypervisorSnapshotReserve != null && hypervisorSnapshotReserve < 0) {
25462553
throw new InvalidParameterValueException("If provided, Hypervisor Snapshot Reserve must be greater than or equal to 0.");
@@ -2799,7 +2806,7 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final List<Long>
27992806
Long bytesWriteRate, Long bytesWriteRateMax, Long bytesWriteRateMaxLength,
28002807
Long iopsReadRate, Long iopsReadRateMax, Long iopsReadRateMaxLength,
28012808
Long iopsWriteRate, Long iopsWriteRateMax, Long iopsWriteRateMaxLength,
2802-
final Integer hypervisorSnapshotReserve) {
2809+
final Integer hypervisorSnapshotReserve, String cacheMode) {
28032810
long diskSize = 0;// special case for custom disk offerings
28042811
if (numGibibytes != null && numGibibytes <= 0) {
28052812
throw new InvalidParameterValueException("Please specify a disk size of at least 1 Gb.");
@@ -2905,6 +2912,9 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final List<Long>
29052912
if (iopsWriteRateMaxLength != null && iopsWriteRateMaxLength > 0) {
29062913
newDiskOffering.setIopsWriteRateMaxLength(iopsWriteRateMaxLength);
29072914
}
2915+
if (cacheMode != null) {
2916+
newDiskOffering.setCacheMode(DiskOffering.DiskCacheMode.valueOf(cacheMode.toUpperCase()));
2917+
}
29082918

29092919
if (hypervisorSnapshotReserve != null && hypervisorSnapshotReserve < 0) {
29102920
throw new InvalidParameterValueException("If provided, Hypervisor Snapshot Reserve must be greater than or equal to 0.");
@@ -2971,6 +2981,9 @@ public DiskOffering createDiskOffering(final CreateDiskOfferingCmd cmd) {
29712981
throw new InvalidParameterValueException("Disksize is not allowed for a customized disk offering");
29722982
}
29732983

2984+
// check if cache_mode parameter is valid
2985+
validateCacheMode(cmd.getCacheMode());
2986+
29742987
boolean localStorageRequired = false;
29752988
final String storageType = cmd.getStorageType();
29762989
if (storageType != null) {
@@ -2997,13 +3010,14 @@ public DiskOffering createDiskOffering(final CreateDiskOfferingCmd cmd) {
29973010
final Long iopsWriteRateMax = cmd.getIopsWriteRateMax();
29983011
final Long iopsWriteRateMaxLength = cmd.getIopsWriteRateMaxLength();
29993012
final Integer hypervisorSnapshotReserve = cmd.getHypervisorSnapshotReserve();
3013+
final String cacheMode = cmd.getCacheMode();
30003014

30013015
final Long userId = CallContext.current().getCallingUserId();
30023016
return createDiskOffering(userId, domainIds, zoneIds, name, description, provisioningType, numGibibytes, tags, isCustomized,
30033017
localStorageRequired, isDisplayOfferingEnabled, isCustomizedIops, minIops,
30043018
maxIops, bytesReadRate, bytesReadRateMax, bytesReadRateMaxLength, bytesWriteRate, bytesWriteRateMax, bytesWriteRateMaxLength,
30053019
iopsReadRate, iopsReadRateMax, iopsReadRateMaxLength, iopsWriteRate, iopsWriteRateMax, iopsWriteRateMaxLength,
3006-
hypervisorSnapshotReserve);
3020+
hypervisorSnapshotReserve, cacheMode);
30073021
}
30083022

30093023
@Override
@@ -6267,6 +6281,15 @@ private List<Long> filterChildSubDomains(final List<Long> domainIds) {
62676281
return filteredDomainIds;
62686282
}
62696283

6284+
protected void validateCacheMode(String cacheMode){
6285+
if(cacheMode != null &&
6286+
!Enums.getIfPresent(DiskOffering.DiskCacheMode.class,
6287+
cacheMode.toUpperCase()).isPresent()) {
6288+
throw new InvalidParameterValueException(String.format("Invalid cache mode (%s). Please specify one of the following " +
6289+
"valid cache mode parameters: none, writeback or writethrough", cacheMode));
6290+
}
6291+
}
6292+
62706293
public List<SecurityChecker> getSecChecker() {
62716294
return _secChecker;
62726295
}

‎test/integration/smoke/test_disk_offerings.py‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,78 @@ def test_05_create_burst_type_disk_offering(self):
223223
)
224224
return
225225

226+
@attr(tags=["advanced", "basic", "eip", "sg", "advancedns", "smoke"], required_hardware="false")
227+
def test_06_create_disk_offering_with_cache_mode_type(self):
228+
"""Test to create disk offering with each one of the valid cache mode types : none, writeback and writethrough
229+
230+
# Validate the following:
231+
# 1. createDiskOfferings should return valid info for new offering
232+
# 2. The Cloud Database contains the valid information
233+
"""
234+
cache_mode_types=["none", "writeback", "writethrough"]
235+
for i in range(3):
236+
disk_offering = DiskOffering.create(
237+
self.apiclient,
238+
self.services["disk_offering"],
239+
cacheMode=cache_mode_types[i]
240+
)
241+
self.cleanup.append(disk_offering)
242+
243+
self.debug("Created Disk offering with valid cacheMode param with ID: %s" % disk_offering.id)
244+
245+
list_disk_response = list_disk_offering(
246+
self.apiclient,
247+
id=disk_offering.id
248+
)
249+
self.assertEqual(
250+
isinstance(list_disk_response, list),
251+
True,
252+
"Check list response returns a valid list"
253+
)
254+
self.assertNotEqual(
255+
len(list_disk_response),
256+
0,
257+
"Check Disk offering is created"
258+
)
259+
disk_response = list_disk_response[0]
260+
261+
self.assertEqual(
262+
disk_response.displaytext,
263+
self.services["disk_offering"]["displaytext"],
264+
"Check server id in createServiceOffering"
265+
)
266+
self.assertEqual(
267+
disk_response.name,
268+
self.services["disk_offering"]["name"],
269+
"Check name in createServiceOffering"
270+
)
271+
self.assertEqual(
272+
disk_response.cacheMode,
273+
cache_mode_types[i],
274+
"Check cacheMode in createServiceOffering"
275+
)
276+
277+
return
278+
279+
@attr(tags=["advanced", "basic", "eip", "sg", "advancedns", "smoke"], required_hardware="false")
280+
def test_07_create_disk_offering_with_invalid_cache_mode_type(self):
281+
"""Test to create disk offering with invalid cacheMode type
282+
283+
# Validate the following:
284+
# 1. createDiskOfferings should return valid info for new offering
285+
# 2. The Cloud Database contains the valid information
286+
"""
287+
288+
with self.assertRaises(Exception):
289+
disk_offering = DiskOffering.create(
290+
self.apiclient,
291+
self.services["disk_offering"],
292+
cacheMode="invalid_cache_mode_type"
293+
)
294+
295+
296+
return
297+
226298
class TestDiskOfferings(cloudstackTestCase):
227299

228300
def setUp(self):

0 commit comments

Comments
 (0)