CLOUDSTACK-5853

Marcus Sorensen · Marcus Sorensen · commit fc082832d3f9 · 2014-01-10T11:55:54.000-07:00
Create two storage pools, one with storage tag X, one with storage tag Y.
Create a service offering with storage tag X.
Create a disk offering with storage tag Y.
Attempt to deploy a virtual machine with a datadisk, using given offerings, it fails.

Deployment planner keeps a global object 'avoid'. It loops through each volume to
be created, asking storage allocators for matching pools, passing this avoid object.

First disk matches a pool or pools, adds ALL other pools to avoid object, then
deployment planner attaches matching pools to a list for that disk.

Second disk matches a pool, adds all other pools to avoid object, then deployment
planner says "wait, matching pool is in avoid, can't use it". Oops. In fact, at this
point ALL pools are in avoid (unless there are other pools that have both tags).

Need to remove matching pool from the avoid set during each select phase.
diff --git a/api/src/com/cloud/deploy/DeploymentPlanner.java b/api/src/com/cloud/deploy/DeploymentPlanner.java
@@ -176,6 +176,12 @@ public void addPool(long poolId) {
             _poolIds.add(poolId);
         }
 
+        public void removePool(long poolId) {
+            if (_poolIds != null) {
+                _poolIds.remove(poolId);
+            }
+        }
+
         public void addDataCenter(long dataCenterId) {
             if (_dcIds == null) {
                 _dcIds = new HashSet<Long>();
diff --git a/engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java b/engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java
@@ -77,14 +77,22 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
         }
 
         List<StoragePoolVO> pools = _storagePoolDao.findPoolsByTags(dcId, podId, clusterId, dskCh.getTags());
+        s_logger.debug("Found pools matching tags: " + pools);
 
         // add remaining pools in cluster, that did not match tags, to avoid set
         List<StoragePoolVO> allPools = _storagePoolDao.findPoolsByTags(dcId, podId, clusterId, null);
         allPools.removeAll(pools);
         for (StoragePoolVO pool : allPools) {
+            s_logger.debug("Adding pool " + pool + " to avoid set since it did not match tags");
             avoid.addPool(pool.getId());
         }
 
+        // make sure our matching pool was not in avoid set
+        for (StoragePoolVO pool : pools) {
+            s_logger.debug("Removing pool " + pool + " from avoid set, must have been inserted when searching for another disk's tag");
+            avoid.removePool(pool.getId());
+        }
+
         if (pools.size() == 0) {
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug("No storage pools available for " + ServiceOffering.StorageType.shared.toString() + " volume allocation, returning");
diff --git a/engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java b/engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java
@@ -89,6 +89,12 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
             avoid.addPool(pool.getId());
         }
 
+        // make sure our matching pool was not in avoid set
+        for (StoragePoolVO pool : storagePoolsByHypervisor) {
+            s_logger.debug("Removing pool " + pool + " from avoid set, must have been inserted when searching for another disk's tag");
+            avoid.removePool(pool.getId());
+        }
+
         for (StoragePoolVO storage : storagePools) {
             if (suitablePools.size() == returnUpTo) {
                 break;
