Sahina Bose has posted comments on this change.

Change subject: webadmin : Gluster Volume Profile Popup
......................................................................


Patch Set 7:

(4 comments)

Please split into separate patches..The TimeUnitconverter can be a separate 
patch, so could the bll and vds command related code.

thanks!

http://gerrit.ovirt.org/#/c/26998/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeProfileInfoQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeProfileInfoQuery.java:

Line 22:     }
Line 23: 
Line 24:     @Override
Line 25:     protected void executeQueryCommand() {
Line 26:         VDSReturnValue nfsReturnValue = 
runVdsCommand(VDSCommandType.GetGlusterVolumeProfileInfo,
Why is this called nfsReturnValue ? 
getParameters()).isNfs() could be false, and then would just be profile info 
from bricks, correct?
Line 27:                 new 
GlusterVolumeProfileInfoVDSParameters(getParameters().getClusterId(),
Line 28:                         getUpServerId(getParameters().getClusterId()),
Line 29:                         
getGlusterVolumeName(getParameters().getVolumeId()),
Line 30:                         
((GlusterVolumeProfileParameters)getParameters()).isNfs()));


Line 31: 
Line 32:         
getQueryReturnValue().setReturnValue((((GlusterVolumeProfileParameters)getParameters()).isNfs())
 ? (GlusterVolumeProfileInfo) nfsReturnValue.getReturnValue() : 
setBrickNames((GlusterVolumeProfileInfo) nfsReturnValue.getReturnValue()));
Line 33:     }
Line 34: 
Line 35:     protected GlusterVolumeProfileInfo 
aggregateBricksAndNfsProfileInfo(GlusterVolumeProfileInfo nfsInfo, 
GlusterVolumeProfileInfo bricksInfo) {
Is this used?
Line 36:         
nfsInfo.setBrickProfileDetails(bricksInfo.getBrickProfileDetails());
Line 37:         return nfsInfo;
Line 38:     }
Line 39: 


http://gerrit.ovirt.org/#/c/26998/7/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/SizeConverter.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/SizeConverter.java:

Line 68:         }
Line 69:         return new Pair<SizeConverter.SizeUnit, 
Double>(SizeUnit.BYTES, (double)size);
Line 70:     }
Line 71: 
Line 72:     public static String autoPreciseConvert(long size, SizeUnit 
inUnit) {
Some docText about what this function does?
Line 73:         String convertedSize = "";
Line 74:         while(size > 1024) {
Line 75:             Pair<SizeUnit, Double> result = 
SizeConverter.autoConvert(size, inUnit);
Line 76:             convertedSize = 
convertedSize.concat(result.getSecond().intValue() + " " + 
result.getFirst().toString() + " ");


http://gerrit.ovirt.org/#/c/26998/7/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;
Could we make this false by default
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: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: anmolbabu <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Sahina Bose <[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

Reply via email to