Shireesh Anjal has posted comments on this change.
Change subject: engine: Get Volume Advanced Details Query
......................................................................
Patch Set 4: (11 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQuery.java
Line 21: @Override
Line 22: protected void executeQueryCommand() {
Line 23: VDSReturnValue returnValue =
getBackendResourceManager().RunVdsCommand(VDSCommandType.GetGlusterVolumeAdvancedDetails,
Line 24: new
GlusterVolumeAdvancedDetailsVDSParameters(getClusterUtils()
Line 25:
.getUpServer(getParameters().getClusterId()).getId(),
getParameters().getClusterId(),
Throw exception if there is no Up server found.
Line 26: getParameters().getVolumeName(),
getParameters().getBrickName(), getParameters().detailsRequired()));
Line 27:
Line 28:
getQueryReturnValue().setReturnValue(returnValue.getReturnValue());
Line 29: }
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQueryTest.java
Line 97: memoryStatus.setMemPool(getMemPoll());
Line 98: return memoryStatus;
Line 99: }
Line 100:
Line 101: private List<Mempool> getMemPoll() {
getMemPoll -> getMemPool ?
Line 102: List<Mempool> memPoolList = new ArrayList<Mempool>();
Line 103: Mempool memPool = new Mempool();
Line 104: memPool.setAllocCount(0);
Line 105: memPool.setColdCount(1024);
Line 134: vds.setstatus(status);
Line 135: return vds;
Line 136: }
Line 137:
Line 138: private List<VDS> mockGetAllVdsForwithStatus(VDSStatus status) {
I don't think this method is doing any mocking. Why not name it according to
what it does e.g. getDummyVdsList(VDSStatus status) ?
Line 139: List<VDS> vdsList = new ArrayList<VDS>();
Line 140: vdsList.add(getVds(status));
Line 141: return vdsList;
Line 142: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/MemoryStatus.java
Line 6: /**
Line 7: * The gluster volume Memory status info.
Line 8: *
Line 9: */
Line 10: public class MemoryStatus{
Use the formatter - space required before {
Line 11:
Line 12: private MallInfo mallInfo;
Line 13:
Line 14: private List<Mempool> memPool;
Line 10: public class MemoryStatus{
Line 11:
Line 12: private MallInfo mallInfo;
Line 13:
Line 14: private List<Mempool> memPool;
memPool*s* ?
Line 15:
Line 16: public MallInfo getMallInfo() {
Line 17: return mallInfo;
Line 18: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/Mempool.java
Line 4: /**
Line 5: * The gluster volume memory status Mem pool.
Line 6: *
Line 7: */
Line 8: public class Mempool{
Use the formatter - space required before {
Line 9:
Line 10: private static final long serialVersionUID = 4426819375609665363L;
Line 11:
Line 12: private String name;
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/ServiceInfo.java
Line 8: */
Line 9: public class ServiceInfo {
Line 10:
Line 11: private boolean nfsServices;
Line 12: private boolean shdServices;
I feel instead of storing boolean variables, we should have an enum, say
ServiceType, and a variable of this type, to identify which service's
information is being stored in the object.
Line 13:
Line 14: private String hostName;
Line 15: private int port;
Line 16: private int pid;
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/gluster/GlusterVolumeAdvancedDetailsParameters.java
Line 7: * This will be used by gluster volume status query. <br>
Line 8: */
Line 9: public class GlusterVolumeAdvancedDetailsParameters extends
GlusterParameters {
Line 10: private static final long serialVersionUID = -1224829720081853632L;
Line 11: private String volumeName;
Add validation annotation to indicate that volumeName is a mandatory field.
Line 12: private String brickName;
Line 13: private boolean detailsRequired;
Line 14:
Line 15: public GlusterVolumeAdvancedDetailsParameters(Guid clusterId,
Line 9: public class GlusterVolumeAdvancedDetailsParameters extends
GlusterParameters {
Line 10: private static final long serialVersionUID = -1224829720081853632L;
Line 11: private String volumeName;
Line 12: private String brickName;
Line 13: private boolean detailsRequired;
Initialize to false by default.
Line 14:
Line 15: public GlusterVolumeAdvancedDetailsParameters(Guid clusterId,
Line 16: String volumeName,
Line 17: String brickName,
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GetGlusterVolumeAdvancedDetailsVDSCommand.java
Line 53: private void executeVolumeStatusInfo(String volumeStatusOption) {
Line 54: result =
getBroker().glusterVolumeStatus(getParameters().getClusterId(),
Line 55: getParameters().getVolumeName(),
getParameters().getBrickName(),
Line 56: volumeStatusOption);
Line 57: ProceedProxyReturnValue();
Please introduce new value in VdcBllErrors for VDSM error related to this
command, and also add handling for it in AbstractGlusterBrokerCommand
Line 58: }
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GlusterVolumeStatusReturnForXmlRpc.java
Line 114: ServiceInfo shdInfo = parseServiceInfo(shdMap);
Line 115: shdInfo.setShdServices(true);
Line 116: serviceInfoList.add(shdInfo);
Line 117: }
Line 118: }
These two if blocks can be reduced to calls to a single method that takes the
service name as argument.
Line 119: return serviceInfoList;
Line 120: }
Line 121:
Line 122: private ServiceInfo parseServiceInfo(Map<String, Object>
volumeServiceInfo) {
--
To view, visit http://gerrit.ovirt.org/7807
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If6b590eaebeb1d06b7278300d5e12b2dab9eb093
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dhandapani Gopal <dgo...@redhat.com>
Gerrit-Reviewer: Dhandapani Gopal <dgo...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Selvasundaram <sesub...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
