From fc082832d3f9741cb06f09593e58db78e8c887d8 Mon Sep 17 00:00:00 2001
From: Marcus Sorensen <marcus@betterservers.com>
Date: Fri, 10 Jan 2014 11:02:35 -0700
Subject: [PATCH] CLOUDSTACK-5853 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.
---
 api/src/com/cloud/deploy/DeploymentPlanner.java           | 6 ++++++
 .../allocator/ClusterScopeStoragePoolAllocator.java       | 8 ++++++++
 .../storage/allocator/ZoneWideStoragePoolAllocator.java   | 6 ++++++
 3 files changed, 20 insertions(+)

diff --git a/api/src/com/cloud/deploy/DeploymentPlanner.java b/api/src/com/cloud/deploy/DeploymentPlanner.java
index 0dccf3d0297..eb62cb178bb 100644
--- a/api/src/com/cloud/deploy/DeploymentPlanner.java
+++ b/api/src/com/cloud/deploy/DeploymentPlanner.java
@@ -176,6 +176,12 @@ public interface DeploymentPlanner extends Adapter {
             _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
index 35737ede5e1..af228107811 100644
--- 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 @@ public class ClusterScopeStoragePoolAllocator extends AbstractStoragePoolAllocat
         }
 
         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
index 57253a787a3..8fb9c8d87ac 100644
--- 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 @@ public class ZoneWideStoragePoolAllocator extends AbstractStoragePoolAllocator {
             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;