Updated Branches:
  refs/heads/master a92095a4a -> 0c1800f70

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

Conflicts:
        
engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java
        
engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/0c1800f7
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/0c1800f7
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/0c1800f7

Branch: refs/heads/master
Commit: 0c1800f70f2faa919ceaf92b00e9edeb79f2c15c
Parents: a92095a
Author: Koushik Das <kous...@apache.org>
Authored: Fri Oct 18 16:34:47 2013 +0530
Committer: Koushik Das <kous...@apache.org>
Committed: Fri Oct 18 17:10:17 2013 +0530

----------------------------------------------------------------------
 .../ClusterScopeStoragePoolAllocator.java       | 19 ++++++-----
 .../allocator/LocalStoragePoolAllocator.java    | 19 ++++++-----
 .../allocator/ZoneWideStoragePoolAllocator.java | 31 +++++++++---------
 .../deploy/DeploymentPlanningManagerImpl.java   | 33 ++++++++++++++++----
 4 files changed, 64 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0c1800f7/engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java
----------------------------------------------------------------------
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 6b076d3..d48edd6 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<StoragePool> 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<StoragePool> suitablePools = new ArrayList<StoragePool>();
 
         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) 
dataStoreMgr.getPrimaryDataStore(pool.getId());
-            if (filter(avoid, pol, dskCh, plan)) {
-                suitablePools.add(pol);
+            StoragePool storagePool = (StoragePool) 
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;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0c1800f7/engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java
----------------------------------------------------------------------
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 3ea2c46..1f61e8b 100644
--- 
a/engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java
+++ 
b/engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java
@@ -68,25 +68,24 @@ public class LocalStoragePoolAllocator extends 
AbstractStoragePoolAllocator {
     @Override
     protected List<StoragePool> select(DiskProfile dskCh, 
VirtualMachineProfile vmProfile,
             DeploymentPlan plan, ExcludeList avoid, int returnUpTo) {
-
-        List<StoragePool> suitablePools = new ArrayList<StoragePool>();
-
         s_logger.debug("LocalStoragePoolAllocator trying to find storage pool 
to fit the vm");
 
         if (!dskCh.useLocalStorage()) {
-            return suitablePools;
+            return null;
         }
 
+        List<StoragePool> suitablePools = new ArrayList<StoragePool>();
+
         // data disk and host identified from deploying vm (attach volume case)
         if (dskCh.getType() == Volume.Type.DATADISK && plan.getHostId() != 
null) {
             List<StoragePoolHostVO> 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());
                     }
@@ -107,9 +106,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());
                 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0c1800f7/engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java
----------------------------------------------------------------------
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 38724fa..b58bcb5 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,25 @@ public class ZoneWideStoragePoolAllocator extends 
AbstractStoragePoolAllocator {
                storageMgr.storagePoolHasEnoughSpace(requestVolumes, pool);
     }
 
-       @Override
-       protected List<StoragePool> select(DiskProfile dskCh,
-                       VirtualMachineProfile vmProfile,
-                       DeploymentPlan plan, ExcludeList avoid, int returnUpTo) 
{
-           s_logger.debug("ZoneWideStoragePoolAllocator to find storage pool");
-               List<StoragePool> suitablePools = new ArrayList<StoragePool>();
 
-        List<StoragePoolVO> storagePools = 
_storagePoolDao.findZoneWideStoragePoolsByTags(plan.getDataCenterId(), 
dskCh.getTags());
+    @Override
+    protected List<StoragePool> 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<StoragePool> suitablePools = new ArrayList<StoragePool>();
 
+        List<StoragePoolVO> storagePools = 
_storagePoolDao.findZoneWideStoragePoolsByTags(plan.getDataCenterId(), 
dskCh.getTags());
         if (storagePools == null) {
             storagePools = new ArrayList<StoragePoolVO>();
         }
 
         List<StoragePoolVO> anyHypervisorStoragePools = new 
ArrayList<StoragePoolVO>();
-
         for (StoragePoolVO storagePool : storagePools) {
             if (HypervisorType.Any.equals(storagePool.getHypervisor())) {
                 anyHypervisorStoragePools.add(storagePool);
@@ -79,9 +83,7 @@ public class ZoneWideStoragePoolAllocator extends 
AbstractStoragePoolAllocator {
         }
 
         List<StoragePoolVO> 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 +97,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<StoragePool> reorderPoolsByNumberOfVolumes(DeploymentPlan 
plan, List<StoragePool> pools,
                                                               Account account) 
{

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0c1800f7/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java 
b/server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java
index dcfb24c..7f616dd 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<StoragePoolVO> 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<StoragePoolVO> 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<StoragePoolVO> 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;
+                    }
+                }
             }
         }
 

Reply via email to