anmolbabu has posted comments on this change. Change subject: gluster: Added additional canDoAction check ......................................................................
Patch Set 4: (5 comments) https://gerrit.ovirt.org/#/c/40118/4//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2015-04-22 12:45:04 +0530 Line 4: Commit: Shubhendu Tripathi <[email protected]> Line 5: CommitDate: 2015-04-22 14:57:43 +0530 Line 6: Line 7: gluster: Added additional canDoAction check may be change the message to something generic (as this also contains a UI change)? Line 8: Line 9: Added additional canDoAction check while creation of Line 10: gluster volume snapshot. If one or more brick(s) are Line 11: down for the volume, snapshot creation would not be https://gerrit.ovirt.org/#/c/40118/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeSnapshotListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeSnapshotListModel.java: Line 163: allowDeactivate = snapshots.get(0).getStatus() == GlusterSnapshotStatus.ACTIVATED; Line 164: } Line 165: } Line 166: Line 167: if (getEntity() == null || getEntity().getStatus() != GlusterStatus.UP) { not required as, changes here have no impact as the instance in VolumeListModel is the actual command being linked to in MainTabVolumeView. Command here is just a proxy(out of vocabulary can't think of a better word ;) ) maintained only to relieve VolumeListModel of additional code(already very bulky you see :)) ) As we discussed may be we need to explore the possibility of uniting(changes in SnapshotListModel relected to the one in VolumeListModel) the proxy here and the actual command in VolumeListModel. This is out of scope of this patch considering the purpose of this patch. But I feel its nice to have it probably in a follow up patch. Line 168: allowCreateSnapshot = false; Line 169: } Line 170: Line 171: if (getEntity() != null && getEntity().getStatus() == GlusterStatus.UP && getEntity().getSnapshotScheduled()) { https://gerrit.ovirt.org/#/c/40118/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java: Line 579: getNewGeoRepSessionCommand().setIsExecutionAllowed(allowCreateGeoRepSession); Line 580: } Line 581: Line 582: private boolean isCreateSnapshotAvailable(List<GlusterVolumeEntity> list) { Line 583: boolean bricksUpFlag = true; is this required Line 584: if (list.size() == 1) { Line 585: List<GlusterBrickEntity> bricks = list.get(0).getBricks(); Line 586: for (GlusterBrickEntity brick : bricks) { Line 587: if (brick.getStatus() != GlusterStatus.UP) { Line 585: List<GlusterBrickEntity> bricks = list.get(0).getBricks(); Line 586: for (GlusterBrickEntity brick : bricks) { Line 587: if (brick.getStatus() != GlusterStatus.UP) { Line 588: bricksUpFlag = false; Line 589: break; why not return false here itself Line 590: } Line 591: } Line 592: Line 593: return (list.get(0).getStatus() == GlusterStatus.UP && bricksUpFlag); Line 589: break; Line 590: } Line 591: } Line 592: Line 593: return (list.get(0).getStatus() == GlusterStatus.UP && bricksUpFlag); and default return w/o flag check Line 594: } else { Line 595: return false; Line 596: } Line 597: } -- To view, visit https://gerrit.ovirt.org/40118 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ed296a6afc4dfa5f3e7023c0dc40b7da4ad4392 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shubhendu Tripathi <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: anmolbabu <[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
