etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Changed the interface in StoragePoolAllocator to avoid a potential NPE in LocalStoragePoolAllocator. Allocators were taking in an instance of VM enclosed inside VirtualMachineProfile. 

However in case of createVolume from Snapshot, there is no VM associated. So VM passed is null and this can cause a NPE.

Allocators hardly use the VM instance. LocalStoragePoolAllocator was mainly using it for checking if host has capacity. But it need not do this check, since that is done by HostAllocators anyway.
So removing the use of VM in StoragePoolAllocators.
This commit is contained in:
prachi 2011-03-09 10:06:53 -08:00
parent 7668e1878a
commit 3624fee85d
9 changed files with 55 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
server/src/com/cloud/deploy/FirstFitPlanner.java
View File

@ -457,7 +457,7 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner {
Enumeration<StoragePoolAllocator> enPool = _storagePoolAllocators.enumeration();
while (enPool.hasMoreElements()) {
final StoragePoolAllocator allocator = enPool.nextElement();
final List<StoragePool> suitablePools = allocator.allocateToPool(diskProfile, vmProfile, plan, avoid, returnUpTo);
final List<StoragePool> suitablePools = allocator.allocateToPool(diskProfile, vmProfile.getTemplate(), plan, avoid, returnUpTo);
if (suitablePools != null && !suitablePools.isEmpty()) {
suitableVolumeStoragePools.put(toBeCreated, suitablePools);
foundPotentialPools = true;

14
server/src/com/cloud/storage/StorageManagerImpl.java
View File

@ -330,13 +330,13 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag
return false;
}
protected StoragePoolVO findStoragePool(DiskProfile dskCh, final DataCenterVO dc, HostPodVO pod, Long clusterId, final ServiceOffering offering,
final VMInstanceVO vm, final VMTemplateVO template, final Set<StoragePool> avoid) {
VirtualMachineProfileImpl<VMInstanceVO> vmProfile = new VirtualMachineProfileImpl<VMInstanceVO>(vm, template, (ServiceOfferingVO)offering, null, null);
protected StoragePoolVO findStoragePool(DiskProfile dskCh, final DataCenterVO dc, HostPodVO pod, Long clusterId,
final VMTemplateVO template, final Set<StoragePool> avoid) {
Enumeration<StoragePoolAllocator> en = _storagePoolAllocators.enumeration();
while (en.hasMoreElements()) {
final StoragePoolAllocator allocator = en.nextElement();
final List<StoragePool> poolList = allocator.allocateToPool(dskCh, vmProfile, dc.getId(), pod.getId(), clusterId, avoid, 1);
final List<StoragePool> poolList = allocator.allocateToPool(dskCh, template, dc.getId(), pod.getId(), clusterId, avoid, 1);
if (poolList != null && !poolList.isEmpty()) {
return (StoragePoolVO) poolList.get(0);
}
@ -439,7 +439,7 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag
while ((pod = _agentMgr.findPod(null, null, dc, account.getId(), podsToAvoid)) != null) {
podsToAvoid.add(pod.first().getId());
// Determine what storage pool to store the volume in
while ((pool = findStoragePool(dskCh, dc, pod.first(), null, null, null, null, poolsToAvoid)) != null) {
while ((pool = findStoragePool(dskCh, dc, pod.first(), null, null, poolsToAvoid)) != null) {
poolsToAvoid.add(pool);
volumeFolder = pool.getPath();
if (s_logger.isDebugEnabled()) {
@ -636,7 +636,7 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag
break;
}
pool = findStoragePool(dskCh, dc, pod, clusterId, offering, vm, template, avoidPools);
pool = findStoragePool(dskCh, dc, pod, clusterId, template, avoidPools);
if (pool == null) {
s_logger.warn("Unable to find storage poll when create volume " + volume.getName());
break;
@ -1317,7 +1317,7 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag
dskCh.setHyperType(dataDiskHyperType);
DataCenterVO destPoolDataCenter = _dcDao.findById(destPoolDcId);
HostPodVO destPoolPod = _podDao.findById(destPoolPodId);
StoragePoolVO destPool = findStoragePool(dskCh, destPoolDataCenter, destPoolPod, destPoolClusterId, null, null, null,
StoragePoolVO destPool = findStoragePool(dskCh, destPoolDataCenter, destPoolPod, destPoolClusterId, null,
new HashSet<StoragePool>());
String secondaryStorageURL = getSecondaryStorageURL(volume.getDataCenterId());
String secondaryStorageVolumePath = null;

17
server/src/com/cloud/storage/allocator/AbstractStoragePoolAllocator.java
View File

@ -54,6 +54,7 @@ import com.cloud.storage.dao.VMTemplateHostDao;
import com.cloud.storage.dao.VMTemplatePoolDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.template.TemplateManager;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.component.AdapterBase;
@ -104,7 +105,7 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement
return true;
}
abstract boolean allocatorIsCorrectType(DiskProfile dskCh, VMInstanceVO vm);
abstract boolean allocatorIsCorrectType(DiskProfile dskCh);
protected boolean templateAvailable(long templateId, long poolId) {
VMTemplateStorageResourceAssoc thvo = _templatePoolDao.findByPoolTemplate(poolId, templateId);
@ -118,12 +119,12 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement
}
}
protected boolean localStorageAllocationNeeded(DiskProfile dskCh, VMInstanceVO vm) {
protected boolean localStorageAllocationNeeded(DiskProfile dskCh) {
return dskCh.useLocalStorage();
}
protected boolean poolIsCorrectType(DiskProfile dskCh, StoragePool pool, VMInstanceVO vm) {
boolean localStorageAllocationNeeded = localStorageAllocationNeeded(dskCh, vm);
protected boolean poolIsCorrectType(DiskProfile dskCh, StoragePool pool) {
boolean localStorageAllocationNeeded = localStorageAllocationNeeded(dskCh);
if (s_logger.isDebugEnabled()) {
s_logger.debug("Is localStorageAllocationNeeded? "+ localStorageAllocationNeeded);
s_logger.debug("Is storage pool shared? "+ pool.getPoolType().isShared());
@ -133,7 +134,7 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement
}
protected boolean checkPool(ExcludeList avoid, StoragePoolVO pool, DiskProfile dskCh, VMTemplateVO template, List<VMTemplateStoragePoolVO> templatesInPool,
VMInstanceVO vm, StatsCollector sc) {
StatsCollector sc) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Checking if storage pool is suitable, name: " + pool.getName()+ " ,poolId: "+ pool.getId());
@ -162,7 +163,7 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement
}
// Check that the pool type is correct
if (!poolIsCorrectType(dskCh, pool, vm)) {
if (!poolIsCorrectType(dskCh, pool)) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("StoragePool is not of correct type, skipping this pool");
}
@ -269,7 +270,7 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement
@Override
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile<? extends VirtualMachine> vm, long dcId, long podId, Long clusterId, Set<? extends StoragePool> avoids, int returnUpTo) {
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineTemplate VMtemplate, long dcId, long podId, Long clusterId, Set<? extends StoragePool> avoids, int returnUpTo) {
ExcludeList avoid = new ExcludeList();
for(StoragePool pool : avoids){
@ -277,7 +278,7 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement
}
DataCenterDeployment plan = new DataCenterDeployment(dcId, podId, clusterId, null);
return allocateToPool(dskCh, vm, plan, avoid, returnUpTo);
return allocateToPool(dskCh, VMtemplate, plan, avoid, returnUpTo);
}
}

14
server/src/com/cloud/storage/allocator/FirstFitStoragePoolAllocator.java
View File

@ -31,6 +31,7 @@ import com.cloud.server.StatsCollector;
import com.cloud.storage.StoragePool;
import com.cloud.storage.StoragePoolVO;
import com.cloud.storage.VMTemplateVO;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.vm.DiskProfile;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
@ -41,21 +42,20 @@ public class FirstFitStoragePoolAllocator extends AbstractStoragePoolAllocator {
private static final Logger s_logger = Logger.getLogger(FirstFitStoragePoolAllocator.class);
@Override
public boolean allocatorIsCorrectType(DiskProfile dskCh, VMInstanceVO vm) {
return !localStorageAllocationNeeded(dskCh, vm);
public boolean allocatorIsCorrectType(DiskProfile dskCh) {
return !localStorageAllocationNeeded(dskCh);
}
@Override
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile<? extends VirtualMachine> vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineTemplate VMtemplate, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
VMInstanceVO vm = (VMInstanceVO)(vmProfile.getVirtualMachine());
VMTemplateVO template = _templateDao.findById(vm.getTemplateId());
VMTemplateVO template = (VMTemplateVO)VMtemplate;
List<StoragePool> suitablePools = new ArrayList<StoragePool>();
// Check that the allocator type is correct
if (!allocatorIsCorrectType(dskCh, vm)) {
if (!allocatorIsCorrectType(dskCh)) {
return suitablePools;
}
long dcId = plan.getDataCenterId();
@ -88,7 +88,7 @@ public class FirstFitStoragePoolAllocator extends AbstractStoragePoolAllocator {
if(suitablePools.size() == returnUpTo){
break;
}
if (checkPool(avoid, pool, dskCh, template, null, vm, sc)) {
if (checkPool(avoid, pool, dskCh, template, null, sc)) {
suitablePools.add(pool);
}
}

10
server/src/com/cloud/storage/allocator/GarbageCollectingStoragePoolAllocator.java
View File

@ -30,6 +30,7 @@ import com.cloud.deploy.DeploymentPlan;
import com.cloud.deploy.DeploymentPlanner.ExcludeList;
import com.cloud.storage.StorageManager;
import com.cloud.storage.StoragePool;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.utils.component.ComponentLocator;
import com.cloud.vm.DiskProfile;
import com.cloud.vm.VMInstanceVO;
@ -47,7 +48,7 @@ public class GarbageCollectingStoragePoolAllocator extends AbstractStoragePoolAl
boolean _storagePoolCleanupEnabled;
@Override
public boolean allocatorIsCorrectType(DiskProfile dskCh, VMInstanceVO vm) {
public boolean allocatorIsCorrectType(DiskProfile dskCh) {
return true;
}
@ -60,7 +61,7 @@ public class GarbageCollectingStoragePoolAllocator extends AbstractStoragePoolAl
}
@Override
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile<? extends VirtualMachine> vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineTemplate VMtemplate, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
if (!_storagePoolCleanupEnabled) {
s_logger.debug("Storage pool cleanup is not enabled, so GarbageCollectingStoragePoolAllocator is being skipped.");
@ -69,10 +70,9 @@ public class GarbageCollectingStoragePoolAllocator extends AbstractStoragePoolAl
// Clean up all storage pools
_storageMgr.cleanupStorage(false);
VMInstanceVO vm = (VMInstanceVO)(vmProfile.getVirtualMachine());
// Determine what allocator to use
StoragePoolAllocator allocator;
if (localStorageAllocationNeeded(dskCh, vm)) {
if (localStorageAllocationNeeded(dskCh)) {
allocator = _localStoragePoolAllocator;
} else {
allocator = _firstFitStoragePoolAllocator;
@ -81,7 +81,7 @@ public class GarbageCollectingStoragePoolAllocator extends AbstractStoragePoolAl
// Try to find a storage pool after cleanup
ExcludeList myAvoids = new ExcludeList(avoid.getDataCentersToAvoid(), avoid.getPodsToAvoid(), avoid.getClustersToAvoid(), avoid.getHostsToAvoid(), avoid.getPoolsToAvoid());
return allocator.allocateToPool(dskCh, vmProfile, plan, myAvoids, returnUpTo);
return allocator.allocateToPool(dskCh, VMtemplate, plan, myAvoids, returnUpTo);
}
@Override

45
server/src/com/cloud/storage/allocator/LocalStoragePoolAllocator.java
View File

@ -38,6 +38,7 @@ import com.cloud.storage.StoragePool;
import com.cloud.storage.StoragePoolHostVO;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.StoragePoolHostDao;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.utils.DateUtil;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.component.Inject;
@ -81,18 +82,17 @@ public class LocalStoragePoolAllocator extends FirstFitStoragePoolAllocator {
@Override
public boolean allocatorIsCorrectType(DiskProfile dskCh, VMInstanceVO vm) {
return localStorageAllocationNeeded(dskCh, vm);
public boolean allocatorIsCorrectType(DiskProfile dskCh) {
return localStorageAllocationNeeded(dskCh);
}
@Override
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile<? extends VirtualMachine> vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineTemplate VMtemplate, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
VMInstanceVO vm = (VMInstanceVO)(vmProfile.getVirtualMachine());
List<StoragePool> suitablePools = new ArrayList<StoragePool>();
// Check that the allocator type is correct
if (!allocatorIsCorrectType(dskCh, vm)) {
if (!allocatorIsCorrectType(dskCh)) {
return suitablePools;
}
@ -103,36 +103,14 @@ public class LocalStoragePoolAllocator extends FirstFitStoragePoolAllocator {
}
List<StoragePool> availablePool;
while (!(availablePool = super.allocateToPool(dskCh, vmProfile, plan, myAvoids, 1)).isEmpty()) {
while (!(availablePool = super.allocateToPool(dskCh, VMtemplate, plan, myAvoids, 1)).isEmpty()) {
StoragePool pool = availablePool.get(0);
myAvoids.addPool(pool.getId());
if (pool.getPoolType().isShared()) {
suitablePools.add(pool);
}else{
List<StoragePoolHostVO> hostsInSPool = _poolHostDao.listByPoolId(pool.getId());
assert(hostsInSPool.size() == 1) : "Local storage pool should be one host per pool";
StoragePoolHostVO spHost = hostsInSPool.get(0);
SearchCriteria<Long> sc = VmsOnPoolSearch.create();
sc.setJoinParameters("volumeJoin", "poolId", pool.getId());
sc.setParameters("state", State.Expunging);
List<Long> vmsOnHost = _vmInstanceDao.customSearchIncludingRemoved(sc, null);
if(s_logger.isDebugEnabled()) {
s_logger.debug("Found " + vmsOnHost.size() + " VM instances are alloacated at host " + spHost.getHostId() + " with local storage pool " + pool.getName());
for(Long vmId : vmsOnHost) {
s_logger.debug("VM " + vmId + " is allocated on host " + spHost.getHostId() + " with local storage pool " + pool.getName());
}
}
if(hostHasCpuMemoryCapacity(spHost.getHostId(), vmsOnHost, vm)) {
s_logger.debug("Found suitable local storage pool " + pool.getId() + ", adding to list");
suitablePools.add(pool);
}else{
s_logger.debug("Found pool " + pool.getId() + " but host doesn't fit, skipping this pool");
}
}
List<StoragePoolHostVO> hostsInSPool = _poolHostDao.listByPoolId(pool.getId());
assert(hostsInSPool.size() == 1) : "Local storage pool should be one host per pool";
s_logger.debug("Found suitable local storage pool " + pool.getId() + ", adding to list");
suitablePools.add(pool);
if(suitablePools.size() == returnUpTo){
break;
@ -151,6 +129,7 @@ public class LocalStoragePoolAllocator extends FirstFitStoragePoolAllocator {
return suitablePools;
}
//we don't need to check host capacity now, since hostAllocators will do that anyway
private boolean hostHasCpuMemoryCapacity(long hostId, List<Long> vmOnHost, VMInstanceVO vm) {
ServiceOffering so = null;

12
server/src/com/cloud/storage/allocator/RandomStoragePoolAllocator.java
View File

@ -31,6 +31,7 @@ import com.cloud.server.StatsCollector;
import com.cloud.storage.StoragePool;
import com.cloud.storage.StoragePoolVO;
import com.cloud.storage.VMTemplateVO;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.vm.DiskProfile;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
@ -41,19 +42,18 @@ public class RandomStoragePoolAllocator extends AbstractStoragePoolAllocator {
private static final Logger s_logger = Logger.getLogger(RandomStoragePoolAllocator.class);
@Override
public boolean allocatorIsCorrectType(DiskProfile dskCh, VMInstanceVO vm) {
public boolean allocatorIsCorrectType(DiskProfile dskCh) {
return true;
}
@Override
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile<? extends VirtualMachine> vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineTemplate VMtemplate, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
List<StoragePool> suitablePools = new ArrayList<StoragePool>();
VMInstanceVO vm = (VMInstanceVO)(vmProfile.getVirtualMachine());
VMTemplateVO template = _templateDao.findById(vm.getTemplateId());
VMTemplateVO template = (VMTemplateVO)VMtemplate;
// Check that the allocator type is correct
if (!allocatorIsCorrectType(dskCh, vm)) {
if (!allocatorIsCorrectType(dskCh)) {
return suitablePools;
}
long dcId = plan.getDataCenterId();
@ -78,7 +78,7 @@ public class RandomStoragePoolAllocator extends AbstractStoragePoolAllocator {
if(suitablePools.size() == returnUpTo){
break;
}
if (checkPool(avoid, pool, dskCh, template, null, vm, sc)) {
if (checkPool(avoid, pool, dskCh, template, null, sc)) {
suitablePools.add(pool);
}
}

5
server/src/com/cloud/storage/allocator/StoragePoolAllocator.java
View File

@ -23,6 +23,7 @@ import com.cloud.deploy.DeploymentPlan;
import com.cloud.deploy.DeploymentPlanner.ExcludeList;
import com.cloud.host.Host;
import com.cloud.storage.StoragePool;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.utils.component.Adapter;
import com.cloud.vm.DiskProfile;
import com.cloud.vm.VirtualMachine;
@ -35,7 +36,7 @@ import com.cloud.vm.VirtualMachineProfile;
public interface StoragePoolAllocator extends Adapter {
//keeping since storageMgr is using this API for some existing functionalities
List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile<? extends VirtualMachine> vm, long dcId, long podId, Long clusterId, Set<? extends StoragePool> avoids, int returnUpTo);
List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineTemplate VMtemplate, long dcId, long podId, Long clusterId, Set<? extends StoragePool> avoids, int returnUpTo);
String chooseStorageIp(VirtualMachine vm, Host host, Host storage);
@ -49,5 +50,5 @@ public interface StoragePoolAllocator extends Adapter {
* @param int returnUpTo (use -1 to return all possible pools)
* @return List<StoragePool> List of storage pools that are suitable for the VM
**/
List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile<? extends VirtualMachine> vm, DeploymentPlan plan, ExcludeList avoid, int returnUpTo);
List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineTemplate VMtemplate, DeploymentPlan plan, ExcludeList avoid, int returnUpTo);
}

9
server/src/com/cloud/storage/allocator/UseLocalForRootAllocator.java
View File

@ -30,6 +30,7 @@ import com.cloud.deploy.DeploymentPlanner.ExcludeList;
import com.cloud.host.Host;
import com.cloud.storage.StoragePool;
import com.cloud.storage.Volume.VolumeType;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.utils.component.ComponentLocator;
import com.cloud.vm.DiskProfile;
import com.cloud.vm.VMInstanceVO;
@ -41,12 +42,12 @@ public class UseLocalForRootAllocator extends LocalStoragePoolAllocator implemen
boolean _useLocalStorage;
@Override
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineProfile<? extends VirtualMachine> vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
public List<StoragePool> allocateToPool(DiskProfile dskCh, VirtualMachineTemplate VMtemplate, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
if (!_useLocalStorage) {
return null;
}
return super.allocateToPool(dskCh, vmProfile, plan, avoid, returnUpTo);
return super.allocateToPool(dskCh, VMtemplate, plan, avoid, returnUpTo);
}
@Override
@ -68,13 +69,13 @@ public class UseLocalForRootAllocator extends LocalStoragePoolAllocator implemen
}
@Override
protected boolean localStorageAllocationNeeded(DiskProfile dskCh, VMInstanceVO vm) {
protected boolean localStorageAllocationNeeded(DiskProfile dskCh) {
if (dskCh.getType() == VolumeType.ROOT) {
return true;
} else if (dskCh.getType() == VolumeType.DATADISK) {
return false;
} else {
return super.localStorageAllocationNeeded(dskCh, vm);
return super.localStorageAllocationNeeded(dskCh);
}
}