-----------------------------------------------------------
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
> 
>

Reply via email to