From 624b324cc1299f93ca2aaf08f8d6484cf39adaa3 Mon Sep 17 00:00:00 2001 From: Koushik Das Date: Fri, 18 Oct 2013 16:34:47 +0530 Subject: [PATCH] CLOUDSTACK-4681: data disk with local disk offering are geting created on shared storage The cluster and zone wide storage pool allocators returned shared pools even for volumes meant to be on local storage pool. If the VM uses local disk then cluster and zone storage allocators should not handle it and return null or empty list. Also fixed the deployment planner to avoid a cluster if a. avoid set returned by storage pool allocators is empty OR b. all local or shared pools in a cluster are in avoid state --- .../ClusterScopeStoragePoolAllocator.java | 19 ++++++----- .../allocator/LocalStoragePoolAllocator.java | 19 +++++------ .../ZoneWideStoragePoolAllocator.java | 30 +++++++++-------- .../deploy/DeploymentPlanningManagerImpl.java | 33 +++++++++++++++---- 4 files changed, 63 insertions(+), 38 deletions(-) diff --git a/engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java b/engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java index a026290c0d7..66d5505b100 100644 --- a/engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java +++ b/engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java @@ -51,8 +51,13 @@ public class ClusterScopeStoragePoolAllocator extends AbstractStoragePoolAllocat @Override protected List select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) { - s_logger.debug("ClusterScopeStoragePoolAllocator looking for storage pool"); + + if (dskCh.useLocalStorage()) { + // cluster wide allocator should bail out in case of local disk + return null; + } + List suitablePools = new ArrayList(); long dcId = plan.getDataCenterId(); @@ -84,9 +89,7 @@ public class ClusterScopeStoragePoolAllocator extends AbstractStoragePoolAllocat if (pools.size() == 0) { if (s_logger.isDebugEnabled()) { - String storageType = dskCh.useLocalStorage() ? ServiceOffering.StorageType.local.toString() - : ServiceOffering.StorageType.shared.toString(); - s_logger.debug("No storage pools available for " + storageType + " volume allocation, returning"); + s_logger.debug("No storage pools available for " + ServiceOffering.StorageType.shared.toString() + " volume allocation, returning"); } return suitablePools; } @@ -95,16 +98,16 @@ public class ClusterScopeStoragePoolAllocator extends AbstractStoragePoolAllocat if (suitablePools.size() == returnUpTo) { break; } - StoragePool pol = (StoragePool) this.dataStoreMgr.getPrimaryDataStore(pool.getId()); - if (filter(avoid, pol, dskCh, plan)) { - suitablePools.add(pol); + StoragePool storagePool = (StoragePool) this.dataStoreMgr.getPrimaryDataStore(pool.getId()); + if (filter(avoid, storagePool, dskCh, plan)) { + suitablePools.add(storagePool); } else { avoid.addPool(pool.getId()); } } if (s_logger.isDebugEnabled()) { - s_logger.debug("FirstFitStoragePoolAllocator returning " + suitablePools.size() + " suitable storage pools"); + s_logger.debug("ClusterScopeStoragePoolAllocator returning " + suitablePools.size() + " suitable storage pools"); } return suitablePools; diff --git a/engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java b/engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java index 58f3c7f88a9..3bc17ea4a66 100644 --- a/engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java +++ b/engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java @@ -67,25 +67,24 @@ public class LocalStoragePoolAllocator extends AbstractStoragePoolAllocator { @Override protected List select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) { - - List suitablePools = new ArrayList(); - s_logger.debug("LocalStoragePoolAllocator trying to find storage pool to fit the vm"); if (!dskCh.useLocalStorage()) { - return suitablePools; + return null; } + List suitablePools = new ArrayList(); + // data disk and host identified from deploying vm (attach volume case) if (dskCh.getType() == Volume.Type.DATADISK && plan.getHostId() != null) { List hostPools = _poolHostDao.listByHostId(plan.getHostId()); for (StoragePoolHostVO hostPool : hostPools) { StoragePoolVO pool = _storagePoolDao.findById(hostPool.getPoolId()); if (pool != null && pool.isLocal()) { - StoragePool pol = (StoragePool) this.dataStoreMgr.getPrimaryDataStore(pool.getId()); - if (filter(avoid, pol, dskCh, plan)) { + StoragePool storagePool = (StoragePool) this.dataStoreMgr.getPrimaryDataStore(pool.getId()); + if (filter(avoid, storagePool, dskCh, plan)) { s_logger.debug("Found suitable local storage pool " + pool.getId() + ", adding to list"); - suitablePools.add(pol); + suitablePools.add(storagePool); } else { avoid.addPool(pool.getId()); } @@ -106,9 +105,9 @@ public class LocalStoragePoolAllocator extends AbstractStoragePoolAllocator { if (suitablePools.size() == returnUpTo) { break; } - StoragePool pol = (StoragePool) this.dataStoreMgr.getPrimaryDataStore(pool.getId()); - if (filter(avoid, pol, dskCh, plan)) { - suitablePools.add(pol); + StoragePool storagePool = (StoragePool) this.dataStoreMgr.getPrimaryDataStore(pool.getId()); + if (filter(avoid, storagePool, dskCh, plan)) { + suitablePools.add(storagePool); } else { avoid.addPool(pool.getId()); } diff --git a/engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java b/engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java index f1723c0fc09..db604c72c04 100644 --- a/engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java +++ b/engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java @@ -57,21 +57,24 @@ public class ZoneWideStoragePoolAllocator extends AbstractStoragePoolAllocator { storageMgr.storagePoolHasEnoughSpace(requestVolumes, pool); } - @Override - protected List select(DiskProfile dskCh, - VirtualMachineProfile vmProfile, - DeploymentPlan plan, ExcludeList avoid, int returnUpTo) { - s_logger.debug("ZoneWideStoragePoolAllocator to find storage pool"); - List suitablePools = new ArrayList(); + @Override + protected List select(DiskProfile dskCh, + VirtualMachineProfile vmProfile, + DeploymentPlan plan, ExcludeList avoid, int returnUpTo) { + s_logger.debug("ZoneWideStoragePoolAllocator to find storage pool"); + + if (dskCh.useLocalStorage()) { + return null; + } + + List suitablePools = new ArrayList(); List storagePools = _storagePoolDao.findZoneWideStoragePoolsByTags(plan.getDataCenterId(), dskCh.getTags()); - if (storagePools == null) { storagePools = new ArrayList(); } List anyHypervisorStoragePools = new ArrayList(); - for (StoragePoolVO storagePool : storagePools) { if (HypervisorType.Any.equals(storagePool.getHypervisor())) { anyHypervisorStoragePools.add(storagePool); @@ -79,9 +82,7 @@ public class ZoneWideStoragePoolAllocator extends AbstractStoragePoolAllocator { } List storagePoolsByHypervisor = _storagePoolDao.findZoneWideStoragePoolsByHypervisor(plan.getDataCenterId(), dskCh.getHypervisorType()); - storagePools.retainAll(storagePoolsByHypervisor); - storagePools.addAll(anyHypervisorStoragePools); // add remaining pools in zone, that did not match tags, to avoid set @@ -95,15 +96,16 @@ public class ZoneWideStoragePoolAllocator extends AbstractStoragePoolAllocator { if (suitablePools.size() == returnUpTo) { break; } - StoragePool pol = (StoragePool) this.dataStoreMgr.getPrimaryDataStore(storage.getId()); - if (filter(avoid, pol, dskCh, plan)) { - suitablePools.add(pol); + StoragePool storagePool = (StoragePool) this.dataStoreMgr.getPrimaryDataStore(storage.getId()); + if (filter(avoid, storagePool, dskCh, plan)) { + suitablePools.add(storagePool); } else { - avoid.addPool(pol.getId()); + avoid.addPool(storagePool.getId()); } } return suitablePools; } + @Override protected List reorderPoolsByNumberOfVolumes(DeploymentPlan plan, List pools, Account account) { diff --git a/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java index 6d36a070291..23622dc5e84 100644 --- a/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -932,15 +932,36 @@ public class DeploymentPlanningManagerImpl extends ManagerBase implements Deploy if (!allocatorAvoidOutput.shouldAvoid(host)) { // there's some host in the cluster that is not yet in avoid set avoidAllHosts = false; + break; } } - List allPoolsInCluster = _storagePoolDao.findPoolsByTags(clusterVO.getDataCenterId(), - clusterVO.getPodId(), clusterVO.getId(), null); - for (StoragePoolVO pool : allPoolsInCluster) { - if (!allocatorAvoidOutput.shouldAvoid(pool)) { - // there's some pool in the cluster that is not yet in avoid set - avoidAllPools = false; + // Cluster can be put in avoid set in following scenarios: + // 1. If storage allocators haven't put any pools in avoid set means either no pools in cluster + // or pools not suitable for the allocators to handle. + // 2. If all 'shared' or 'local' pools are in avoid set + if (allocatorAvoidOutput.getPoolsToAvoid() != null && !allocatorAvoidOutput.getPoolsToAvoid().isEmpty()) { + // check shared pools + List allPoolsInCluster = _storagePoolDao.findPoolsByTags(clusterVO.getDataCenterId(), + clusterVO.getPodId(), clusterVO.getId(), null); + for (StoragePoolVO pool : allPoolsInCluster) { + if (!allocatorAvoidOutput.shouldAvoid(pool)) { + // there's some pool in the cluster that is not yet in avoid set + avoidAllPools = false; + break; + } + } + if (avoidAllPools) { + // check local pools + List allLocalPoolsInCluster = _storagePoolDao.findLocalStoragePoolsByTags(clusterVO.getDataCenterId(), + clusterVO.getPodId(), clusterVO.getId(), null); + for (StoragePoolVO pool : allLocalPoolsInCluster) { + if (!allocatorAvoidOutput.shouldAvoid(pool)) { + // there's some pool in the cluster that is not yet in avoid set + avoidAllPools = false; + break; + } + } } }