GaOrtiga commented on code in PR #7214:
URL: https://github.com/apache/cloudstack/pull/7214#discussion_r1409230421


##########
server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java:
##########
@@ -808,109 +850,101 @@ public void 
checkForNonDedicatedResources(VirtualMachineProfile vmProfile, DataC
 
         //Only when the type is instance VM and not explicitly dedicated.
         if (vm.getType() == VirtualMachine.Type.User && !isExplicit) {
-            //add explicitly dedicated resources in avoidList
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Adding pods to avoid lists for non-explicit VM 
deployment: " + allPodsInDc);
-            }
-            avoids.addPodList(allPodsInDc);
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Adding clusters to avoid lists for 
non-explicit VM deployment: " + allClustersInDc);
-            }
-            avoids.addClusterList(allClustersInDc);
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Adding hosts to avoid lists for non-explicit 
VM deployment: " + allHostsInDc);
-            }
-            avoids.addHostList(allHostsInDc);
+            findAvoidSetForNonExplicitUserVM(avoids, isExplicit, vm, 
allPodsInDc, allClustersInDc, allHostsInDc);
         }
 
         //Handle the Virtual Router Case
         //No need to check the isExplicit. As both the cases are handled.
         if (vm.getType() == VirtualMachine.Type.DomainRouter) {
-            long vmAccountId = vm.getAccountId();
-            long vmDomainId = vm.getDomainId();
-
-            //Lists all explicitly dedicated resources from vm account ID or 
domain ID.
-            List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
-            List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
-            List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
+            findAvoiSetForRouterVM(avoids, vm, allPodsInDc, allClustersInDc, 
allHostsInDc);
+        }
+    }
 
-            //Whether the dedicated resources belong to Domain or not. If not, 
it may belongs to Account or no dedication.
-            List<AffinityGroupDomainMapVO> domainGroupMappings = 
_affinityGroupDomainMapDao.listByDomain(vmDomainId);
+    private void findAvoiSetForRouterVM(ExcludeList avoids, VirtualMachine vm, 
List<Long> allPodsInDc, List<Long> allClustersInDc, List<Long> allHostsInDc) {
+        long vmAccountId = vm.getAccountId();
+        long vmDomainId = vm.getDomainId();
 
-            //For temporary storage and indexing.
-            List<DedicatedResourceVO> tempStorage;
+        List<Long> allPodsFromDedicatedID = new ArrayList<Long>();
+        List<Long> allClustersFromDedicatedID = new ArrayList<Long>();
+        List<Long> allHostsFromDedicatedID = new ArrayList<Long>();
 
-            if (domainGroupMappings == null || domainGroupMappings.isEmpty()) {
-                //The dedicated resource belongs to VM Account ID.
+        List<AffinityGroupDomainMapVO> domainGroupMappings = 
_affinityGroupDomainMapDao.listByDomain(vmDomainId);
 
-                tempStorage = _dedicatedDao.searchDedicatedPods(null, 
vmDomainId, vmAccountId, null, new Filter(DedicatedResourceVO.class, "id", 
true, 0L, 1L)).first();
+        List<DedicatedResourceVO> tempStorage;
 
-                for(DedicatedResourceVO vo : tempStorage) {
-                    allPodsFromDedicatedID.add(vo.getPodId());
-                }
+        if (domainGroupMappings == null || domainGroupMappings.isEmpty()) {
+            tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, 
vmAccountId, null,
+                    new Filter(DedicatedResourceVO.class, "id", true, 0L, 
1L)).first();
 
-                tempStorage.clear();
-                tempStorage = _dedicatedDao.searchDedicatedClusters(null, 
vmDomainId, vmAccountId, null, new Filter(DedicatedResourceVO.class, "id", 
true, 0L, 1L)).first();
+            for (DedicatedResourceVO vo : tempStorage) {
+                allPodsFromDedicatedID.add(vo.getPodId());
+            }
 
-                for(DedicatedResourceVO vo : tempStorage) {
-                    allClustersFromDedicatedID.add(vo.getClusterId());
-                }
+            tempStorage.clear();
+            tempStorage = _dedicatedDao.searchDedicatedClusters(null, 
vmDomainId, vmAccountId, null,
+                    new Filter(DedicatedResourceVO.class, "id", true, 0L, 
1L)).first();
 
-                tempStorage.clear();
-                tempStorage = _dedicatedDao.searchDedicatedHosts(null, 
vmDomainId, vmAccountId, null, new Filter(DedicatedResourceVO.class, "id", 
true, 0L, 1L)).first();
+            for (DedicatedResourceVO vo : tempStorage) {
+                allClustersFromDedicatedID.add(vo.getClusterId());
+            }
 
-                for(DedicatedResourceVO vo : tempStorage) {
-                    allHostsFromDedicatedID.add(vo.getHostId());
-                }
+            tempStorage.clear();
+            tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, 
vmAccountId, null,
+                    new Filter(DedicatedResourceVO.class, "id", true, 0L, 
1L)).first();
 
-                //Remove the dedicated ones from main list
-                allPodsInDc.removeAll(allPodsFromDedicatedID);
-                allClustersInDc.removeAll(allClustersFromDedicatedID);
-                allHostsInDc.removeAll(allHostsFromDedicatedID);
+            for (DedicatedResourceVO vo : tempStorage) {
+                allHostsFromDedicatedID.add(vo.getHostId());
             }
-            else {
-                //The dedicated resource belongs to VM Domain ID or No 
dedication.
-
-                tempStorage = _dedicatedDao.searchDedicatedPods(null, 
vmDomainId, null, null, new Filter(DedicatedResourceVO.class, "id", true, 0L, 
1L)).first();
 
-                for(DedicatedResourceVO vo : tempStorage) {
-                    allPodsFromDedicatedID.add(vo.getPodId());
-                }
+            allPodsInDc.removeAll(allPodsFromDedicatedID);
+            allClustersInDc.removeAll(allClustersFromDedicatedID);
+            allHostsInDc.removeAll(allHostsFromDedicatedID);
+        } else {
+            tempStorage = _dedicatedDao.searchDedicatedPods(null, vmDomainId, 
null, null,
+                    new Filter(DedicatedResourceVO.class, "id", true, 0L, 
1L)).first();
 
-                tempStorage.clear();
-                tempStorage = _dedicatedDao.searchDedicatedClusters(null, 
vmDomainId, null, null, new Filter(DedicatedResourceVO.class, "id", true, 0L, 
1L)).first();
+            for (DedicatedResourceVO vo : tempStorage) {
+                allPodsFromDedicatedID.add(vo.getPodId());
+            }
 
-                for(DedicatedResourceVO vo : tempStorage) {
-                    allClustersFromDedicatedID.add(vo.getClusterId());
-                }
+            tempStorage.clear();
+            tempStorage = _dedicatedDao.searchDedicatedClusters(null, 
vmDomainId, null, null,
+                    new Filter(DedicatedResourceVO.class, "id", true, 0L, 
1L)).first();
 
-                tempStorage.clear();
-                tempStorage = _dedicatedDao.searchDedicatedHosts(null, 
vmDomainId, null, null, new Filter(DedicatedResourceVO.class, "id", true, 0L, 
1L)).first();
+            for (DedicatedResourceVO vo : tempStorage) {
+                allClustersFromDedicatedID.add(vo.getClusterId());
+            }
 
-                for(DedicatedResourceVO vo : tempStorage) {
-                    allHostsFromDedicatedID.add(vo.getHostId());
-                }
+            tempStorage.clear();
+            tempStorage = _dedicatedDao.searchDedicatedHosts(null, vmDomainId, 
null, null,
+                    new Filter(DedicatedResourceVO.class, "id", true, 0L, 
1L)).first();
 
-                //Remove the dedicated ones from main list
-                allPodsInDc.removeAll(allPodsFromDedicatedID);
-                allClustersInDc.removeAll(allClustersFromDedicatedID);
-                allHostsInDc.removeAll(allHostsFromDedicatedID);
+            for (DedicatedResourceVO vo : tempStorage) {
+                allHostsFromDedicatedID.add(vo.getHostId());
             }
 
-            //Add in avoid list or no addition if no dedication
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Adding pods to avoid lists: " + allPodsInDc);
-            }
-            avoids.addPodList(allPodsInDc);
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Adding clusters to avoid lists: " + 
allClustersInDc);
-            }
-            avoids.addClusterList(allClustersInDc);
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Adding hosts to avoid lists: " + allHostsInDc);
-            }
-            avoids.addHostList(allHostsInDc);
+            allPodsInDc.removeAll(allPodsFromDedicatedID);
+            allClustersInDc.removeAll(allClustersFromDedicatedID);
+            allHostsInDc.removeAll(allHostsFromDedicatedID);
         }
+
+        s_logger.debug(LogUtils.logGsonWithoutException(
+                "Adding pods [%s], clusters [%s] and hosts [%s] to the avoid 
list in the deploy process of VR VM [%s], "
+                        + "because this VM is not dedicated to this 
components.",
+                allPodsInDc, allClustersInDc, allHostsInDc, vm.getUuid()));
+        avoids.addPodList(allPodsInDc);
+        avoids.addClusterList(allClustersInDc);
+        avoids.addHostList(allHostsInDc);
+    }
+
+    private void findAvoidSetForNonExplicitUserVM(ExcludeList avoids, boolean 
isExplicit, VirtualMachine vm, List<Long> allPodsInDc, List<Long> 
allClustersInDc, List<Long> allHostsInDc) {
+        s_logger.debug(LogUtils.logGsonWithoutException(
+                "Adding pods [%s], clusters [%s] and hosts [%s] to the avoid 
list in the deploy process of user VM [%s], "
+                        + "because this VM is not explicit dedicated to this 
components.",

Review Comment:
   ```suggestion
                           + "because this VM is not explicitly dedicated to 
these components.",
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to