Updated Branches: refs/heads/4.1 7d877cea8 -> 75832c893
CLOUDSTACK-2281: Fix NPE in the planner, in the case that pool ID cannot be looked up in database when deploying a VM This fixes a null pointer if selected pool on which to deploy is unable to be looked up via id. Used the same coding style found in the original code (large if/else block). If this is accepted, I'll fix master too. This patch doesn't apply cleanly, but the code is very similar. Testing: Fixed broken system, tested against devcloud to ensure that it didn't break a stock/fresh install (advanced zone with 3 storage pools). Restarted VMs to ensure that the planner still functioned that way as well. Signed-off-by: Chip Childers <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/75832c89 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/75832c89 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/75832c89 Branch: refs/heads/4.1 Commit: 75832c893b342c905d692e893a46af3dfd838943 Parents: 7d877ce Author: Marcus Sorensen <[email protected]> Authored: Tue Apr 30 15:19:30 2013 +0100 Committer: Chip Childers <[email protected]> Committed: Tue Apr 30 15:19:30 2013 +0100 ---------------------------------------------------------------------- server/src/com/cloud/deploy/FirstFitPlanner.java | 42 +++++++++------- 1 files changed, 24 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/75832c89/server/src/com/cloud/deploy/FirstFitPlanner.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/deploy/FirstFitPlanner.java b/server/src/com/cloud/deploy/FirstFitPlanner.java index 31fc43c..c49d25e 100755 --- a/server/src/com/cloud/deploy/FirstFitPlanner.java +++ b/server/src/com/cloud/deploy/FirstFitPlanner.java @@ -746,37 +746,43 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { //If the plan specifies a poolId, it means that this VM's ROOT volume is ready and the pool should be reused. //In this case, also check if rest of the volumes are ready and can be reused. if(plan.getPoolId() != null){ - s_logger.debug("Volume has pool already allocated, checking if pool can be reused, poolId: "+toBeCreated.getPoolId()); + s_logger.debug("Volume has pool(" + plan.getPoolId() + ") already allocated, checking if pool can be reused, poolId: "+toBeCreated.getPoolId()); List<StoragePool> suitablePools = new ArrayList<StoragePool>(); StoragePoolVO pool; if(toBeCreated.getPoolId() != null){ + s_logger.debug("finding pool by id '" + toBeCreated.getPoolId() + "'"); pool = _storagePoolDao.findById(toBeCreated.getPoolId()); }else{ + s_logger.debug("finding pool by id '" + plan.getPoolId() + "'"); pool = _storagePoolDao.findById(plan.getPoolId()); } - - if(!pool.isInMaintenance()){ - if(!avoid.shouldAvoid(pool)){ - long exstPoolDcId = pool.getDataCenterId(); - - long exstPoolPodId = pool.getPodId() != null ? pool.getPodId() : -1; - long exstPoolClusterId = pool.getClusterId() != null ? pool.getClusterId() : -1; - if(plan.getDataCenterId() == exstPoolDcId && plan.getPodId() == exstPoolPodId && plan.getClusterId() == exstPoolClusterId){ - s_logger.debug("Planner need not allocate a pool for this volume since its READY"); - suitablePools.add(pool); - suitableVolumeStoragePools.put(toBeCreated, suitablePools); - if (!(toBeCreated.getState() == Volume.State.Allocated || toBeCreated.getState() == Volume.State.Creating)) { - readyAndReusedVolumes.add(toBeCreated); + + if(pool != null){ + if(!pool.isInMaintenance()){ + if(!avoid.shouldAvoid(pool)){ + long exstPoolDcId = pool.getDataCenterId(); + + long exstPoolPodId = pool.getPodId() != null ? pool.getPodId() : -1; + long exstPoolClusterId = pool.getClusterId() != null ? pool.getClusterId() : -1; + if(plan.getDataCenterId() == exstPoolDcId && plan.getPodId() == exstPoolPodId && plan.getClusterId() == exstPoolClusterId){ + s_logger.debug("Planner need not allocate a pool for this volume since its READY"); + suitablePools.add(pool); + suitableVolumeStoragePools.put(toBeCreated, suitablePools); + if (!(toBeCreated.getState() == Volume.State.Allocated || toBeCreated.getState() == Volume.State.Creating)) { + readyAndReusedVolumes.add(toBeCreated); + } + continue; + }else{ + s_logger.debug("Pool of the volume does not fit the specified plan, need to reallocate a pool for this volume"); } - continue; }else{ - s_logger.debug("Pool of the volume does not fit the specified plan, need to reallocate a pool for this volume"); + s_logger.debug("Pool of the volume is in avoid set, need to reallocate a pool for this volume"); } }else{ - s_logger.debug("Pool of the volume is in avoid set, need to reallocate a pool for this volume"); + s_logger.debug("Pool of the volume is in maintenance, need to reallocate a pool for this volume"); } }else{ - s_logger.debug("Pool of the volume is in maintenance, need to reallocate a pool for this volume"); + s_logger.debug("Unable to find pool by provided id"); } }
