----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6781/#review11575 -----------------------------------------------------------
1) long dcId = dest.getDataCenter().getId(); List<HostPodVO> pods = _podDao.listByDataCenterIdVMTypeAndState(dcId, VirtualMachine.Type.User, VirtualMachine.State.Running); pods.addAll(_podDao.listByDataCenterIdVMTypeAndState(dcId, VirtualMachine.Type.User, VirtualMachine.State.Starting)); Instead of calling method 2 times, you can add State... to the istByDataCenterIdVMTypeAndState method and pass both states - Starting and Running - separated by coma. Please return list of PodIds in the response; we don't need the objects. The dao method has to be placed 2) The above applies to listUserVms as well: List<DomainRouterVO> virtualRouters = _routerDao.listByPodIdAndState(podId, VirtualMachine.State.Running); virtualRouters.addAll(_routerDao.listByPodIdAndState(podId, VirtualMachine.State.Starting)); Call one dao method, and only get the count(). 3) Move this method to vmInstanceDaoImpl - it doesn't make any sense to keep it in podDaoImpl as all we have to return - podIds, and the original search builder is done against vmInstanceDao. And vm_instance table has pod_id/data_center_id fields, there is no reason to make a joins with host_pod_ref.I Also pass zoneId, vmState(s) - make it accept multiple state by defining State... in the method signature, vmType parameters to the call: @Override public List<HostPodVO> listByDataCenterIdVMTypeAndState(long id, VirtualMachine.Type type, VirtualMachine.State state) { final VMInstanceDaoImpl _vmDao = ComponentLocator.inject(VMInstanceDaoImpl.class); SearchBuilder<VMInstanceVO> vmInstanceSearch = _vmDao.createSearchBuilder(); vmInstanceSearch.and("type", vmInstanceSearch.entity().getType(), SearchCriteria.Op.EQ); vmInstanceSearch.and("state", vmInstanceSearch.entity().getState(), SearchCriteria.Op.EQ); SearchBuilder<HostPodVO> podIdSearch = createSearchBuilder(); podIdSearch.and("dc", podIdSearch.entity().getDataCenterId(), SearchCriteria.Op.EQ); podIdSearch.select(null, SearchCriteria.Func.DISTINCT, podIdSearch.entity().getId()); podIdSearch.join("vmInstanceSearch", vmInstanceSearch, podIdSearch.entity().getId(), vmInstanceSearch.entity().getPodIdToDeployIn(), JoinBuilder.JoinType.INNER); podIdSearch.done(); SearchCriteria<HostPodVO> sc = podIdSearch.create(); sc.setParameters("dc", id); sc.setJoinParameters("vmInstanceSearch", "type", type); sc.setJoinParameters("vmInstanceSearch", "state", state); return listBy(sc); } 4) Remove this method from domainRouterDaoImpl: @Override + public List<DomainRouterVO> listByPodId(Long podId) { + SearchCriteria<DomainRouterVO> sc = AllFieldsSearch.create(); + sc.setParameters("podId", podId); + return listBy(sc); + } This method can cover both cases - search by pod and search by podId - if State is changed to State... in the method signature: @Override + public List<DomainRouterVO> listByPodIdAndState(Long podId, State state) { + SearchCriteria<DomainRouterVO> sc = AllFieldsSearch.create(); + sc.setParameters("podId", podId); + sc.setParameters("state", state); + return listBy(sc); + } State... can accept either none, or one, or multiple values for state. - Alena Prokharchyk On Sept. 12, 2012, 11:32 a.m., Rohit Yadav wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6781/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2012, 11:32 a.m.) > > > Review request for cloudstack, Abhinandan Prateek, Alena Prokharchyk, and > Chiradeep Vittal. > > > Description > ------- > > Patch as per discussion on ML and http://bugs.cloudstack.org/browse/CS-16104 > CLOUDSTACK-70 on issues.apache.org > > Download original patch: http://patchbin.baagi.org/p?id=yfawv5 > git am <patch> # to apply retaining commit message > > > This addresses bug CS-16104. > > > Diffs > ----- > > server/src/com/cloud/dc/dao/HostPodDao.java f031ac9 > server/src/com/cloud/dc/dao/HostPodDaoImpl.java bec8c51 > server/src/com/cloud/network/NetworkManagerImpl.java 292a259 > server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java > 6618fdf > server/src/com/cloud/vm/dao/DomainRouterDao.java 01e3258 > server/src/com/cloud/vm/dao/DomainRouterDaoImpl.java 175d3f2 > server/src/com/cloud/vm/dao/VMInstanceDao.java 2cf3d75 > server/src/com/cloud/vm/dao/VMInstanceDaoImpl.java 571b5d1 > ui/scripts/network.js d0f65c4 > > Diff: https://reviews.apache.org/r/6781/diff/ > > > Testing > ------- > > > Thanks, > > Rohit Yadav > >