anmolbabu has posted comments on this change. Change subject: engine : Query to fetch list of volumes eligible for geo replication ......................................................................
Patch Set 7: (5 comments) http://gerrit.ovirt.org/#/c/33845/7/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 42: Set<GlusterVolumeEntity> volumesAlreadySlaveOfGeoRepSessions = new HashSet<GlusterVolumeEntity>(); Line 43: for(GlusterGeoRepSession session : getDbFacade().getGlusterGeoRepDao().getAllSessions()) { Line 44: String slaveVolumeName = session.getSlaveVolumeName(); Line 45: String slaveHostName = session.getSlaveHostName(); Line 46: GlusterVolumeEntity slaveVolume = getGlusterVolumeDao().getByName(getVdsDao().getByName(slaveHostName).getVdsGroupId(), slaveVolumeName); > possible NPE here. I think you could use session.getSlaveVolumeId() instead Done Line 47: volumesAlreadySlaveOfGeoRepSessions.add(slaveVolume); Line 48: } Line 49: return volumesAlreadySlaveOfGeoRepSessions; Line 50: } Line 56: */ Line 57: Line 58: private Set<GlusterVolumeEntity> getUpVolumesWithSuggestedCapacityAndNotInMasterCluster(GlusterVolumeEntity masterVolume) { Line 59: Set<GlusterVolumeEntity> eligibleVolumeList = new HashSet<GlusterVolumeEntity>(); Line 60: List<VDSGroup> vdsGroups = getVdsGroupDao().getAll(); > Why don't you get vdsGroups that supports gluster service and with compatib Done Line 61: VDSGroup masterVdsGroup = getVdsGroupDao().get(masterVolume.getClusterId()); Line 62: for (VDSGroup vdsGroup : vdsGroups) { Line 63: if (vdsGroup.getId() != masterVolume.getClusterId() && vdsGroup.getcompatibility_version().equals(masterVdsGroup.getcompatibility_version()) && vdsGroup.supportsGlusterService()) { Line 64: List<GlusterVolumeEntity> volumes = getGlusterVolumeDao().getByClusterId(vdsGroup.getId()); Line 59: Set<GlusterVolumeEntity> eligibleVolumeList = new HashSet<GlusterVolumeEntity>(); Line 60: List<VDSGroup> vdsGroups = getVdsGroupDao().getAll(); Line 61: VDSGroup masterVdsGroup = getVdsGroupDao().get(masterVolume.getClusterId()); Line 62: for (VDSGroup vdsGroup : vdsGroups) { Line 63: if (vdsGroup.getId() != masterVolume.getClusterId() && vdsGroup.getcompatibility_version().equals(masterVdsGroup.getcompatibility_version()) && vdsGroup.supportsGlusterService()) { > !vdsGroup.getId().equals(masterVolume.getClusterId()) oops ok Line 64: List<GlusterVolumeEntity> volumes = getGlusterVolumeDao().getByClusterId(vdsGroup.getId()); Line 65: for (GlusterVolumeEntity volume : volumes) { Line 66: if (volume.getStatus() == GlusterStatus.UP) { Line 67: //Sync Job to fetch capacity details may not have run yet. Line 86: */ Line 87: Line 88: private List<GlusterVolumeEntity> getEligibleVolumesList(Set<GlusterVolumeEntity> upVolumesWithSuggestedCapacityAndNotInMasterCluster, Set<GlusterVolumeEntity> existingSessionSlaveVolumes) { Line 89: List<GlusterVolumeEntity> eligibleVolumes = new ArrayList<GlusterVolumeEntity>(); Line 90: upVolumesWithSuggestedCapacityAndNotInMasterCluster.removeAll(existingSessionSlaveVolumes); > you could just return upVolumesWithSuggestedCapacityAndNotInMasterCluster h Done Line 91: eligibleVolumes.addAll(upVolumesWithSuggestedCapacityAndNotInMasterCluster); Line 92: return eligibleVolumes; Line 93: } http://gerrit.ovirt.org/#/c/33845/7/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterGeoRepDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterGeoRepDaoDbFacadeImpl.java: Line 195: } Line 196: Line 197: @Override Line 198: public List<GlusterGeoRepSession> getAllSessions() { Line 199: return getCallsHandler().executeReadList("GetAllGlusterGeoRepSessions", georepSessionRowMapper, getCustomMapSqlParameterSource().addValue("user_id", null).addValue("is_filtered", false)); > user_id and is_filtered parameters not required Done.I actually copied it from some query and modified it probably missed this. Line 200: } Line 201: -- 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: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: anmolbabu <anb...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Ramesh N <rnach...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: anmolbabu <anb...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
