anmolbabu has posted comments on this change. Change subject: engine : Refactored gluster volume profile VdsCommand ......................................................................
Patch Set 15: (5 comments) http://gerrit.ovirt.org/#/c/26998/15/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterVolumeProfileInfo.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterVolumeProfileInfo.java: Line 49: if(!(obj instanceof GlusterVolumeProfileInfo)) { Line 50: return false; Line 51: } Line 52: GlusterVolumeProfileInfo profileInfo = (GlusterVolumeProfileInfo) obj; Line 53: if(!ObjectUtils.objectsEqual(getVolumeId(), profileInfo.getVolumeId())){ > no need to check here that the lists are equal as well? Done Line 54: return false; Line 55: } Line 56: return true; Line 57: } Line 59: @Override Line 60: public int hashCode() { Line 61: final int prime = 31; Line 62: int result = 1; Line 63: result = prime * result + ((getVolumeId() == null) ? 0 : getVolumeId().hashCode()); > same about hash code? Done Line 64: return result; Line 65: } http://gerrit.ovirt.org/#/c/26998/15/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/NfsProfileDetails.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/NfsProfileDetails.java: Line 30: } Line 31: public void setStatsInfo(List<StatsInfo> statsInfo) { Line 32: this.statsInfo = statsInfo; Line 33: } Line 34: public static long getSerialversionuid() { > why do you need this method? Sorry while generating the getters and setters for each of the fields.This seems to have been created by mistake.I'll remove it right now. Line 35: return serialVersionUID; Line 36: } Line 37: Line 38: @Override Line 40: if(!(obj instanceof NfsProfileDetails)) { Line 41: return false; Line 42: } Line 43: NfsProfileDetails nfsDetails = (NfsProfileDetails) obj; Line 44: if(!ObjectUtils.objectsEqual(getNfsServerIp(), nfsDetails.getNfsServerIp())) { > same question about the list here Done Line 45: return false; Line 46: } Line 47: return true; Line 48: } http://gerrit.ovirt.org/#/c/26998/15/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/gluster/GlusterVolumeProfileInfoVDSParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/gluster/GlusterVolumeProfileInfoVDSParameters.java: Line 7: * This will be used directly by Gluster Volume Profile Info Query. Line 8: */ Line 9: public class GlusterVolumeProfileInfoVDSParameters extends GlusterVolumeVDSParameters { Line 10: private Guid clusterId; Line 11: private boolean nfs; > if this is not NFS what other option do you have? is boolean sufficient her yes boolean is sufficient bcoz if its not nfs it should be a brick profile case. i.e, gluster volume profile command can be used 1.with an optional arguement nfs in which case the command returns the fop and block stats for each of the nfs servers which are contributing to the bricks of the volume on which profile command is fired. 2.If nfs argument is not passed to the command the command returns brick-wise fop and block stats of the volume on which profile command was fired. In our vdsm,we have a single verb that takes an optional boolean arguement nfs For case 1, we pass true.When we pass true,the vds concatenates nfs as an arguement to the volume profile command and fires it For case 2, we pass false.Vdsm fires a plain volume profile command without nfs parameter in which case we get brick-wise stats. Line 12: Line 13: public GlusterVolumeProfileInfoVDSParameters(Guid clusterId, Guid serverId, String volumeName, boolean nfs) { Line 14: super(serverId, volumeName); Line 15: this.clusterId = clusterId; -- To view, visit http://gerrit.ovirt.org/26998 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I26cda9f8603e77296720f2dc062b94d5c10e7fd1 Gerrit-PatchSet: 15 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: Yair Zaslavsky <yzasl...@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
