Sahina Bose has posted comments on this change. Change subject: engine : VdsCommand for geo-rep status detail query ......................................................................
Patch Set 7: (3 comments) http://gerrit.ovirt.org/#/c/30472/7/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GlusterVolumeGeoRepStatusDetailForXmlRpc.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GlusterVolumeGeoRepStatusDetailForXmlRpc.java: Line 31: private static final String FILES_SKIPPED = "filesSkipped"; Line 32: private static final String GEO_REP_PAIRS = "pairs"; Line 33: Line 34: private List<GlusterGeoRepSessionDetails> geoRepDetails = new ArrayList<GlusterGeoRepSessionDetails>(); Line 35: private List<GlusterGeoRepSession> geoRepSessions = new ArrayList<GlusterGeoRepSession>(); there would only be a single session returned from georepStatusDetail, no? Line 36: Line 37: private String masterVolumeName; Line 38: private String sessionKey; Line 39: Line 76: details.setDeletesPending(deletesPending); Line 77: details.setSessionId(sessionKey); Line 78: Line 79: session.setMasterVolumeId(dbUtils.getVolumeByNameAndHostId(masterVolumeName, masterNodeId).getId()); Line 80: session.setSlaveHostName(slaveHost); This is incorrect. the session should be retrieved from the database, or if not available, the slave host and volume should be populated based on the info present in the sessionKey The session object should be populated in the parent method and each detail associated with it. Line 81: session.setSlaveVolumeName(slaveVolume); Line 82: session.setSessionKey(sessionKey); Line 83: } Line 84: Line 89: Line 90: if(innerMap.containsKey(GEO_REP_PAIRS)) { Line 91: for(Object sessionPair : (Object[]) innerMap.get(GEO_REP_PAIRS)) { Line 92: GlusterGeoRepSessionDetails details = new GlusterGeoRepSessionDetails(); Line 93: GlusterGeoRepSession session = new GlusterGeoRepSession(); Why is the session object being initialized for every pair? A session is associated with multiple session details. In this case, it looks like each detail object is associated with a new session? Line 94: populatePairDetails((Map<String, Object>) sessionPair, details, session); Line 95: geoRepDetails.add(details); Line 96: geoRepSessions.add(session); Line 97: } -- To view, visit http://gerrit.ovirt.org/30472 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b8236e7e516eba46d183c3f83a973175edba90f 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
