Kanagaraj M has posted comments on this change. Change subject: gluster: BLL command to set volume snapshot config ......................................................................
Patch Set 2: (3 comments) http://gerrit.ovirt.org/#/c/36294/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/UpdateGlusterVolumeSnapshotConfigCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/UpdateGlusterVolumeSnapshotConfigCommand.java: Line 49: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_CLUSTER_IS_NOT_VALID); Line 50: return false; Line 51: } Line 52: Line 53: if (getParameters() == null || getParameters().getConfigParams() == null getParameters() == null is redundant check as you have already used getParameters().getClusterId() Line 54: || getParameters().getConfigParams().size() == 0) { Line 55: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_GLUSTER_VOLUME_SNAPSHOT_CONFIG_PARAMS_IS_EMPTY); Line 56: return false; Line 57: } Line 87: } Line 88: Line 89: List<GlusterVolumeSnapshotConfig> configParams = getParameters().getConfigParams(); Line 90: Line 91: // segregate the cluster and volume specific config params why not just send them as two different lists? Line 92: Map<String, GlusterVolumeSnapshotConfig> clusterConfigParams = Line 93: new HashMap<String, GlusterVolumeSnapshotConfig>(); Line 94: Map<String, GlusterVolumeSnapshotConfig> volumeConfigParams = Line 95: new HashMap<String, GlusterVolumeSnapshotConfig>(); Line 106: new ArrayList<GlusterVolumeSnapshotConfig>(); Line 107: for (GlusterVolumeSnapshotConfig cfgParam : clusterConfigParams.values()) { Line 108: GlusterVolumeSnapshotConfig fetchedCfgParam = fetchedClusterConfigParams.get(cfgParam.getParamName()); Line 109: if (!(fetchedCfgParam.getParamValue().equals(cfgParam.getParamValue()))) { Line 110: updatedClusterConfigParams.add(cfgParam); Why do we need to fetch and update, instead of simply updating it directly? Line 111: } Line 112: } Line 113: List<GlusterVolumeSnapshotConfig> updatedVolumeConfigParams = Line 114: new ArrayList<GlusterVolumeSnapshotConfig>(); -- To view, visit http://gerrit.ovirt.org/36294 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I37b6f4dd02ca4258e5e40e7da11f0a657ac6b330 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shubhendu Tripathi <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[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
