Sahina Bose has posted comments on this change.
Change subject: gluster: bll command to start/stop/restart service
......................................................................
Patch Set 4: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/ManageGlusterServiceCommand.java
Line 64:
Line 65: public ManageGlusterServiceCommand(GlusterServiceParameters
params) {
Line 66: super(params);
Line 67: this.clusterId = params.getClusterId();
Line 68: this.serverId = params.getServerId();
also call setVdsGroupId based on parameters here for up server check in Gluster
command base
Line 69: this.serviceType = params.getServiceType();
Line 70: this.actionType = params.getActionType();
Line 71: }
Line 72:
Line 121: }
Line 122:
Line 123: private void performActionForServicesOfCluster() {
Line 124: List<VDS> servers =
getClusterUtils().getAllUpServers(clusterId);
Line 125: final List<GlusterService> serviceList = getServiceList();
getServiceList can return a List<String> which returns the service names alone.
You could then reuse it in performActionForServicesOfServer as well
Line 126:
Line 127: List<Callable<Pair<VDS, VDSReturnValue>>> taskList = new
ArrayList<Callable<Pair<VDS, VDSReturnValue>>>();
Line 128: for (final VDS upServer : servers) {
Line 129: final List<String> serviceListStr = new
ArrayList<String>();
Line 146: }
Line 147: });
Line 148: }
Line 149:
Line 150: setSucceeded(true);
You don't need this as you're setting succeeded below in the code based on
errors.size
Line 151: if (!taskList.isEmpty()) {
Line 152: List<Pair<VDS, VDSReturnValue>> pairResults =
ThreadPoolUtil.invokeAll(taskList);
Line 153: for (Pair<VDS, VDSReturnValue> pairResult : pairResults) {
Line 154: VDSReturnValue retValue = pairResult.getSecond();
Line 152: List<Pair<VDS, VDSReturnValue>> pairResults =
ThreadPoolUtil.invokeAll(taskList);
Line 153: for (Pair<VDS, VDSReturnValue> pairResult : pairResults) {
Line 154: VDSReturnValue retValue = pairResult.getSecond();
Line 155: if (!retValue.getSucceeded()) {
Line 156: errors.add(retValue.getVdsError().getMessage());
You could move this to inside the inner method call() - so you do not need to
iterate over pairResults again
Line 157: }
Line 158: }
Line 159: } else {
Line 160: setSucceeded(false);
Line 189: returnValue.getVdsError().getMessage());
Line 190: } else {
Line 191: VDS vds = new VDS();
Line 192: vds.setStatus(VDSStatus.Up);
Line 193: Pair<VDS, VDSReturnValue> pairRetVal = new Pair<VDS,
VDSReturnValue>(vds, returnValue);
Consider refactoring updateService to take serverId, and
List<GlusterServerService>. Do not create a VDS here.
Line 194: updateService(serverId, serviceList, pairRetVal);
Line 195: }
Line 196: }
Line 197:
--
To view, visit http://gerrit.ovirt.org/14831
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcab38866c49c6f5d43e3b33006c428ec9304501
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
