Skip to content

Commit d140123

Browse files
author
Mike Tutkowski
committed
Changes per Rafael's review comments
1 parent 4270bd4 commit d140123

File tree

6 files changed

+33
-31
lines changed

6 files changed

+33
-31
lines changed

‎engine/components-api/src/main/java/com/cloud/storage/StorageUtil.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ private Long getClusterId(Long hostId) {
5353
return hostVO.getClusterId();
5454
}
5555

56+
/**
57+
* This method relates to managed storage only. CloudStack currently supports managed storage with XenServer, vSphere, and KVM.
58+
* With managed storage on XenServer and vSphere, CloudStack needs to use an iSCSI SR (XenServer) or datastore (vSphere) per CloudStack
59+
* volume. Since XenServer and vSphere are limited to the hundreds with regards to how many SRs or datastores can be leveraged per
60+
* compute cluster, this method is used to check a Global Setting (that specifies the maximum number of SRs or datastores per compute cluster)
61+
* against what is being requested.
62+
*
63+
* If the clusterId is passed in, we use it. Otherwise, we use the hostId. If neither leads to a cluster, we just return true.
64+
*/
5665
public boolean managedStoragePoolCanScale(StoragePool storagePool, Long clusterId, Long hostId) {
5766
if (clusterId == null) {
5867
clusterId = getClusterId(hostId);

‎engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,13 @@
5555
public abstract class AbstractStoragePoolAllocator extends AdapterBase implements StoragePoolAllocator {
5656
private static final Logger s_logger = Logger.getLogger(AbstractStoragePoolAllocator.class);
5757

58-
protected BigDecimal _storageOverprovisioningFactor = new BigDecimal(1);
58+
protected BigDecimal storageOverprovisioningFactor = new BigDecimal(1);
5959
protected DataStoreManager dataStoreMgr;
60-
protected String _allocationAlgorithm = "random";
61-
@Inject PrimaryDataStoreDao _storagePoolDao;
60+
protected String allocationAlgorithm = "random";
61+
@Inject PrimaryDataStoreDao storagePoolDao;
6262
@Inject VolumeDao volumeDao;
6363
@Inject ConfigurationDao configDao;
64-
long _extraBytesPerVolume = 0;
64+
long extraBytesPerVolume = 0;
6565
@Inject private CapacityDao capacityDao;
6666
@Inject private ClusterDao clusterDao;
6767
@Inject private StorageManager storageMgr;
@@ -73,11 +73,11 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
7373
if(configDao != null) {
7474
Map<String, String> configs = configDao.getConfiguration(null, params);
7575
String globalStorageOverprovisioningFactor = configs.get("storage.overprovisioning.factor");
76-
_storageOverprovisioningFactor = new BigDecimal(NumbersUtil.parseFloat(globalStorageOverprovisioningFactor, 2.0f));
77-
_extraBytesPerVolume = 0;
76+
storageOverprovisioningFactor = new BigDecimal(NumbersUtil.parseFloat(globalStorageOverprovisioningFactor, 2.0f));
77+
extraBytesPerVolume = 0;
7878
String allocationAlgorithm = configs.get("vm.allocation.algorithm");
7979
if (allocationAlgorithm != null) {
80-
_allocationAlgorithm = allocationAlgorithm;
80+
this.allocationAlgorithm = allocationAlgorithm;
8181
}
8282
return true;
8383
}
@@ -163,12 +163,12 @@ protected List<StoragePool> reOrder(List<StoragePool> pools, VirtualMachineProfi
163163
account = vmProfile.getOwner();
164164
}
165165

166-
if (_allocationAlgorithm.equals("random") || _allocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) {
166+
if (allocationAlgorithm.equals("random") || allocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) {
167167
// Shuffle this so that we don't check the pools in the same order.
168168
Collections.shuffle(pools);
169-
} else if (_allocationAlgorithm.equals("userdispersing")) {
169+
} else if (allocationAlgorithm.equals("userdispersing")) {
170170
pools = reorderPoolsByNumberOfVolumes(plan, pools, account);
171-
} else if(_allocationAlgorithm.equals("firstfitleastconsumed")){
171+
} else if(allocationAlgorithm.equals("firstfitleastconsumed")){
172172
pools = reorderPoolsByCapacity(plan, pools);
173173
}
174174
return pools;
@@ -206,7 +206,7 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh,
206206
return false;
207207
}
208208

209-
if (pool.isManaged() && !managedStoragePoolCanScale(pool, plan)) {
209+
if (pool.isManaged() && !storageUtil.managedStoragePoolCanScale(pool, plan.getClusterId(), plan.getHostId())) {
210210
return false;
211211
}
212212

@@ -217,10 +217,6 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh,
217217
return storageMgr.storagePoolHasEnoughIops(requestVolumes, pool) && storageMgr.storagePoolHasEnoughSpace(requestVolumes, pool, plan.getClusterId());
218218
}
219219

220-
private boolean managedStoragePoolCanScale(StoragePool storagePool, DeploymentPlan plan) {
221-
return storageUtil.managedStoragePoolCanScale(storagePool, plan.getClusterId(), plan.getHostId());
222-
}
223-
224220
/*
225221
Check StoragePool and Volume type compatibility for the hypervisor
226222
*/

‎engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,19 +70,19 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
7070

7171
if (s_logger.isTraceEnabled()) {
7272
// Log the pools details that are ignored because they are in disabled state
73-
List<StoragePoolVO> disabledPools = _storagePoolDao.findDisabledPoolsByScope(dcId, podId, clusterId, ScopeType.CLUSTER);
73+
List<StoragePoolVO> disabledPools = storagePoolDao.findDisabledPoolsByScope(dcId, podId, clusterId, ScopeType.CLUSTER);
7474
if (disabledPools != null && !disabledPools.isEmpty()) {
7575
for (StoragePoolVO pool : disabledPools) {
7676
s_logger.trace("Ignoring pool " + pool + " as it is in disabled state.");
7777
}
7878
}
7979
}
8080

81-
List<StoragePoolVO> pools = _storagePoolDao.findPoolsByTags(dcId, podId, clusterId, dskCh.getTags());
81+
List<StoragePoolVO> pools = storagePoolDao.findPoolsByTags(dcId, podId, clusterId, dskCh.getTags());
8282
s_logger.debug("Found pools matching tags: " + pools);
8383

8484
// add remaining pools in cluster, that did not match tags, to avoid set
85-
List<StoragePoolVO> allPools = _storagePoolDao.findPoolsByTags(dcId, podId, clusterId, null);
85+
List<StoragePoolVO> allPools = storagePoolDao.findPoolsByTags(dcId, podId, clusterId, null);
8686
allPools.removeAll(pools);
8787
for (StoragePoolVO pool : allPools) {
8888
s_logger.debug("Adding pool " + pool + " to avoid set since it did not match tags");
@@ -123,7 +123,7 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
123123
Map<String, String> configs = configDao.getConfiguration(params);
124124
String allocationAlgorithm = configs.get("vm.allocation.algorithm");
125125
if (allocationAlgorithm != null) {
126-
_allocationAlgorithm = allocationAlgorithm;
126+
this.allocationAlgorithm = allocationAlgorithm;
127127
}
128128
}
129129
return true;

‎engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
6969

7070
if (s_logger.isTraceEnabled()) {
7171
// Log the pools details that are ignored because they are in disabled state
72-
List<StoragePoolVO> disabledPools = _storagePoolDao.findDisabledPoolsByScope(plan.getDataCenterId(), plan.getPodId(), plan.getClusterId(), ScopeType.HOST);
72+
List<StoragePoolVO> disabledPools = storagePoolDao.findDisabledPoolsByScope(plan.getDataCenterId(), plan.getPodId(), plan.getClusterId(), ScopeType.HOST);
7373
if (disabledPools != null && !disabledPools.isEmpty()) {
7474
for (StoragePoolVO pool : disabledPools) {
7575
s_logger.trace("Ignoring pool " + pool + " as it is in disabled state.");
@@ -81,7 +81,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
8181

8282
// data disk and host identified from deploying vm (attach volume case)
8383
if (plan.getHostId() != null) {
84-
List<StoragePoolVO> hostTagsPools = _storagePoolDao.findLocalStoragePoolsByHostAndTags(plan.getHostId(), dskCh.getTags());
84+
List<StoragePoolVO> hostTagsPools = storagePoolDao.findLocalStoragePoolsByHostAndTags(plan.getHostId(), dskCh.getTags());
8585
for (StoragePoolVO pool : hostTagsPools) {
8686
if (pool != null && pool.isLocal()) {
8787
StoragePool storagePool = (StoragePool)this.dataStoreMgr.getPrimaryDataStore(pool.getId());
@@ -103,7 +103,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
103103
return null;
104104
}
105105
List<StoragePoolVO> availablePools =
106-
_storagePoolDao.findLocalStoragePoolsByTags(plan.getDataCenterId(), plan.getPodId(), plan.getClusterId(), dskCh.getTags());
106+
storagePoolDao.findLocalStoragePoolsByTags(plan.getDataCenterId(), plan.getPodId(), plan.getClusterId(), dskCh.getTags());
107107
for (StoragePoolVO pool : availablePools) {
108108
if (suitablePools.size() == returnUpTo) {
109109
break;
@@ -118,7 +118,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
118118

119119
// add remaining pools in cluster, that did not match tags, to avoid
120120
// set
121-
List<StoragePoolVO> allPools = _storagePoolDao.findLocalStoragePoolsByTags(plan.getDataCenterId(), plan.getPodId(), plan.getClusterId(), null);
121+
List<StoragePoolVO> allPools = storagePoolDao.findLocalStoragePoolsByTags(plan.getDataCenterId(), plan.getPodId(), plan.getClusterId(), null);
122122
allPools.removeAll(availablePools);
123123
for (StoragePoolVO pool : allPools) {
124124
avoid.addPool(pool.getId());
@@ -136,8 +136,8 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
136136
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
137137
super.configure(name, params);
138138

139-
_storageOverprovisioningFactor = new BigDecimal(1);
140-
_extraBytesPerVolume = NumbersUtil.parseLong((String)params.get("extra.bytes.per.volume"), 50 * 1024L * 1024L);
139+
storageOverprovisioningFactor = new BigDecimal(1);
140+
extraBytesPerVolume = NumbersUtil.parseLong((String)params.get("extra.bytes.per.volume"), 50 * 1024L * 1024L);
141141

142142
return true;
143143
}

‎engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.springframework.stereotype.Component;
2828

2929
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
30-
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
3130
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
3231

3332
import com.cloud.deploy.DeploymentPlan;
@@ -43,8 +42,6 @@
4342
public class ZoneWideStoragePoolAllocator extends AbstractStoragePoolAllocator {
4443
private static final Logger LOGGER = Logger.getLogger(ZoneWideStoragePoolAllocator.class);
4544
@Inject
46-
private PrimaryDataStoreDao storagePoolDao;
47-
@Inject
4845
private DataStoreManager dataStoreMgr;
4946

5047

@@ -100,7 +97,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
10097
if (filter(avoid, storagePool, dskCh, plan)) {
10198
suitablePools.add(storagePool);
10299
} else {
103-
if (isAddStoragePoolToAvoidSet(storage)) {
100+
if (canAddStoragePoolToAvoidSet(storage)) {
104101
avoid.addPool(storagePool.getId());
105102
}
106103
}
@@ -109,7 +106,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
109106
}
110107

111108
// Don't add zone-wide, managed storage to the avoid list because it may be usable for another cluster.
112-
private boolean isAddStoragePoolToAvoidSet(StoragePoolVO storagePoolVO) {
109+
private boolean canAddStoragePoolToAvoidSet(StoragePoolVO storagePoolVO) {
113110
return !ScopeType.ZONE.equals(storagePoolVO.getScope()) || !storagePoolVO.isManaged();
114111
}
115112

‎plugins/storage-allocators/random/src/main/java/org/apache/cloudstack/storage/allocator/RandomStoragePoolAllocator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfi
4848
}
4949

5050
s_logger.debug("Looking for pools in dc: " + dcId + " pod:" + podId + " cluster:" + clusterId);
51-
List<StoragePoolVO> pools = _storagePoolDao.listBy(dcId, podId, clusterId, ScopeType.CLUSTER);
51+
List<StoragePoolVO> pools = storagePoolDao.listBy(dcId, podId, clusterId, ScopeType.CLUSTER);
5252
if (pools.size() == 0) {
5353
if (s_logger.isDebugEnabled()) {
5454
s_logger.debug("No storage pools available for allocation, returning");

0 commit comments

Comments
 (0)