From b593657503c4743c6e74c76c083df744ed0bb2ae Mon Sep 17 00:00:00 2001
From: Sudhansu <sudhansu.sahu@citrix.com>
Date: Thu, 12 Mar 2015 17:58:31 +0530
Subject: [PATCH] BUG-ID: CLOUDSTACK-8484 - Hosts without tag are not listed
 while listing the hosts for migration for instance with tag

While preparing the suitable hosts we are accidentally removing the
incompatible (host does not have host tag) hosts from otherhost list(
incorrect use of List.retainAll).
---
 .../allocator/impl/RandomAllocator.java       | 13 +++++----
 .../allocator/impl/FirstFitAllocator.java     | 28 +++++++++++++++----
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/plugins/host-allocators/random/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java b/plugins/host-allocators/random/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java
index 5bdf4f1beaf..390a8afd089 100755
--- a/plugins/host-allocators/random/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java
+++ b/plugins/host-allocators/random/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java
@@ -61,6 +61,7 @@ public class RandomAllocator extends AdapterBase implements HostAllocator {
         Long clusterId = plan.getClusterId();
         ServiceOffering offering = vmProfile.getServiceOffering();
         List<Host> suitableHosts = new ArrayList<Host>();
+        List<Host> hostsCopy = new ArrayList<Host>(hosts);
 
         if (type == Host.Type.Storage) {
             return suitableHosts;
@@ -75,18 +76,18 @@ public class RandomAllocator extends AdapterBase implements HostAllocator {
 
         // list all computing hosts, regardless of whether they support routing...it's random after all
         if (hostTag != null) {
-            hosts.retainAll(_hostDao.listByHostTag(type, clusterId, podId, dcId, hostTag));
+            hostsCopy.retainAll(_hostDao.listByHostTag(type, clusterId, podId, dcId, hostTag));
         } else {
-            hosts.retainAll(_resourceMgr.listAllUpAndEnabledHosts(type, clusterId, podId, dcId));
+            hostsCopy.retainAll(_resourceMgr.listAllUpAndEnabledHosts(type, clusterId, podId, dcId));
         }
 
-        s_logger.debug("Random Allocator found " + hosts.size() + "  hosts");
-        if (hosts.size() == 0) {
+        s_logger.debug("Random Allocator found " + hostsCopy.size() + "  hosts");
+        if (hostsCopy.size() == 0) {
             return suitableHosts;
         }
 
-        Collections.shuffle(hosts);
-        for (Host host : hosts) {
+        Collections.shuffle(hostsCopy);
+        for (Host host : hostsCopy) {
             if (suitableHosts.size() == returnUpTo) {
                 break;
             }
diff --git a/server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java b/server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
index a746eb756ca..990b77bc780 100755
--- a/server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
+++ b/server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
@@ -192,6 +192,7 @@ public class FirstFitAllocator extends AdapterBase implements HostAllocator {
         VMTemplateVO template = (VMTemplateVO)vmProfile.getTemplate();
         Account account = vmProfile.getOwner();
         List<Host> suitableHosts = new ArrayList<Host>();
+        List<Host> hostsCopy = new ArrayList<Host>(hosts);
 
         if (type == Host.Type.Storage) {
             // FirstFitAllocator should be used for user VMs only since it won't care whether the host is capable of
@@ -206,23 +207,38 @@ public class FirstFitAllocator extends AdapterBase implements HostAllocator {
 
         String haVmTag = (String)vmProfile.getParameter(VirtualMachineProfile.Param.HaTag);
         if (haVmTag != null) {
-            hosts.retainAll(_hostDao.listByHostTag(type, clusterId, podId, dcId, haVmTag));
+            hostsCopy.retainAll(_hostDao.listByHostTag(type, clusterId, podId, dcId, haVmTag));
         } else {
             if (hostTagOnOffering == null && hostTagOnTemplate == null) {
-                hosts.retainAll(_resourceMgr.listAllUpAndEnabledNonHAHosts(type, clusterId, podId, dcId));
+                hostsCopy.retainAll(_resourceMgr.listAllUpAndEnabledNonHAHosts(type, clusterId, podId, dcId));
             } else {
                 if (hasSvcOfferingTag) {
-                    hosts.retainAll(_hostDao.listByHostTag(type, clusterId, podId, dcId, hostTagOnOffering));
+                    if (s_logger.isDebugEnabled()) {
+                        s_logger.debug("Looking for hosts having tag specified on SvcOffering:" + hostTagOnOffering);
+                    }
+                    hostsCopy.retainAll(_hostDao.listByHostTag(type, clusterId, podId, dcId, hostTagOnOffering));
+
+                    if (s_logger.isDebugEnabled()) {
+                        s_logger.debug("Hosts with tag '" + hostTagOnOffering + "' are:" + hostsCopy);
+                    }
                 }
 
                 if (hasTemplateTag) {
-                    hosts.retainAll(_hostDao.listByHostTag(type, clusterId, podId, dcId, hostTagOnTemplate));
+                    if (s_logger.isDebugEnabled()) {
+                        s_logger.debug("Looking for hosts having tag specified on Template:" + hostTagOnTemplate);
+                    }
+
+                    hostsCopy.retainAll(_hostDao.listByHostTag(type, clusterId, podId, dcId, hostTagOnTemplate));
+
+                    if (s_logger.isDebugEnabled()) {
+                        s_logger.debug("Hosts with tag '" + hostTagOnTemplate + "' are:" + hostsCopy);
+                    }
                 }
             }
         }
 
-        if (!hosts.isEmpty()) {
-            suitableHosts = allocateTo(plan, offering, template, avoid, hosts, returnUpTo, considerReservedCapacity, account);
+        if (!hostsCopy.isEmpty()) {
+            suitableHosts = allocateTo(plan, offering, template, avoid, hostsCopy, returnUpTo, considerReservedCapacity, account);
         }
 
         return suitableHosts;