Yair Zaslavsky has posted comments on this change.
Change subject: engine: Get Volume Advanced Details Query
......................................................................
Patch Set 9: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/GlusterVolumeAdvancedDetails.java
Line 59: }
Line 60: }
Line 61:
Line 62: private BrickDetails copyBrickDetails(BrickDetails brick,
BrickDetails entityBrick) {
Line 63: brick.setBrickProperties(entityBrick.getBrickProperties());
Notice that two BrickDetails object point to same brick properties object.
Is this ok?
Line 64: return brick;
Line 65: }
Line 66:
Line 67: public void copyClientsFrom(GlusterVolumeAdvancedDetails entity) {
Line 74: }
Line 75: }
Line 76:
Line 77: private BrickDetails copyClientDetails(BrickDetails brick,
BrickDetails entityBrick) {
Line 78: brick.setClients(entityBrick.getClients());
Same about clientDetails (see previous question)
Line 79: return brick;
Line 80: }
Line 81:
Line 82: public void copyMemoryFrom(GlusterVolumeAdvancedDetails entity) {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/MallInfo.java
Line 10: public class MallInfo implements Serializable {
Line 11:
Line 12: private static final long serialVersionUID = -5333436625976301182L;
Line 13:
Line 14: private Integer arena;
All the Integer stuff - is this on purpose? can they be nullable?
Line 15:
Line 16: private Integer ordblks;
Line 17:
Line 18: private Integer smblks;
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/Mempool.java
Line 11:
Line 12: private static final long serialVersionUID = 4426819375609665363L;
Line 13:
Line 14: private String name;
Line 15:
Same question as before about Integer
Line 16: private Integer hotCount;
Line 17:
Line 18: private Integer coldCount;
Line 19:
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GetGlusterVolumeAdvancedDetailsVDSCommand.java
Line 32: executeVolumeStatusInfo("");
Line 33: if (getSucceeded()) {
Line 34: volumeAdvancedDetails =
result.getVolumeAdvancedDetails();
Line 35:
executeVolumeStatusInfo(GlusterVolumeStatusOption.detail.name());
Line 36: if (getSucceeded()) {
Why can't it be in the same nesting level as the previous if?
if the previous if evaluated as false, this "if" also evaluates as "false".
In other cases, the evaluation of this if depends on the execution of
"executeVolumeStatusInfo".
Am I missing here something?
Line 37:
volumeAdvancedDetails.copyDetailsFrom(result.getVolumeAdvancedDetails());
Line 38:
executeVolumeStatusInfo(GlusterVolumeStatusOption.clients.name());
Line 39: if (getSucceeded()) {
Line 40:
volumeAdvancedDetails.copyClientsFrom(result.getVolumeAdvancedDetails());
--
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: 9
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
