anmolbabu has posted comments on this change. Change subject: engine : Query to fetch list of volumes eligible for geo replication ......................................................................
Patch Set 6: (3 comments) http://gerrit.ovirt.org/#/c/33845/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetEligibleVolumesForGeoRepEnablingQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetEligibleVolumesForGeoRepEnablingQuery.java: Line 24: for (VDSGroup vdsGroup : vdsGroups) { Line 25: if (vdsGroup.getId() != masterVolume.getClusterId() && vdsGroup.getcompatibility_version().equals(getDbFacade().getVdsGroupDao().get(masterVolume.getClusterId()).getcompatibility_version()) && vdsGroup.supportsGlusterService()) { Line 26: List<GlusterVolumeEntity> volumes = getGlusterVolumeDao().getByClusterId(vdsGroup.getId()); Line 27: for (GlusterVolumeEntity volume : volumes) { Line 28: if (volume.getStatus() == GlusterStatus.UP && volume.getAdvancedDetails().getCapacityInfo().getUsedSize().equals(0L) && volume.getAdvancedDetails().getCapacityInfo().getFreeSize() >= masterVolume.getAdvancedDetails().getCapacityInfo().getUsedSize()) { > Yes -- I think that's the criteria that geo-rep currently has. That capacit Done Line 29: eligibleVolumeList.add(volume); Line 30: } Line 31: } Line 32: } http://gerrit.ovirt.org/#/c/33845/6/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetEligibleVolumesForGeoRepEnablingQueryTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetEligibleVolumesForGeoRepEnablingQueryTest.java: Line 79: cluster1.setStoragePoolId(DC_1); Line 80: cluster1.setGlusterService(false); Line 81: cluster1.setId(CLUSTER_ID_1); Line 82: cluster1.setcompatibility_version(new Version("3.4")); Line 83: clusters.add(cluster1); > you could refactor getMasterVDS to take parameters to avoid repeating code Done Line 84: Line 85: clusters.add(getMasterVds()); Line 86: Line 87: VDSGroup cluster3 = new VDSGroup(); Line 153: doReturn(getGlusterVolume(VOLUME_ID_1, CLUSTER_ID_1, GlusterStatus.UP, new GlusterVolumeSizeInfo(10000L, 6000L, 4000L))).when(volumeDao).getById(VOLUME_ID_1); Line 154: doReturn(getGlusterVolume(VOLUME_ID_2, CLUSTER_ID_2, GlusterStatus.UP, new GlusterVolumeSizeInfo(10000L, 10000L, 0L))).when(volumeDao).getById(VOLUME_ID_2); Line 155: doReturn(getGlusterVolume(VOLUME_ID_3, MASTER_CLUSTER_ID, GlusterStatus.UP, new GlusterVolumeSizeInfo())).when(volumeDao).getById(VOLUME_ID_3); Line 156: doReturn(getGlusterVolume(VOLUME_ID_4, CLUSTER_ID_1, GlusterStatus.UP, new GlusterVolumeSizeInfo(10000L, 10000L, 0L))).when(volumeDao).getById(VOLUME_ID_4); Line 157: doReturn(getGlusterVolume(VOLUME_ID_5, CLUSTER_ID_2, GlusterStatus.UP, new GlusterVolumeSizeInfo(10000L, 10000L, 0L))).when(volumeDao).getById(VOLUME_ID_5); > Do you need to add a test where volumeAdvancedDetails is null for a volume? GlusterVolumeAdvancedDetails is never null within a volume as it is instantiated within the constructor of GlusterVolumeEntity. GlusterVolumeSize Info can be null however. I'll add a test for this. Line 158: Line 159: doReturn(getFirstClusterVolumes()).when(volumeDao).getByClusterId(CLUSTER_ID_1); Line 160: doReturn(getMasterClusterVolumes()).when(volumeDao).getByClusterId(MASTER_CLUSTER_ID); Line 161: doReturn(getSecondClusterVolumes()).when(volumeDao).getByClusterId(CLUSTER_ID_2); -- To view, visit http://gerrit.ovirt.org/33845 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fc3ecb15535181f1ca2a8780461cb89788a3f41 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: anmolbabu <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Shubhendu Tripathi <[email protected]> Gerrit-Reviewer: anmolbabu <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
