Sahina Bose has posted comments on this change.

Change subject: gluster: start/stop gluster service
......................................................................


Patch Set 1: (5 inline comments)

Shubhendu, my comments inline

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/ManageGlusterServiceCommand.java
Line 61: 
Line 62:     @Override
Line 63:     protected boolean canDoAction() {
Line 64:         serverService = 
getGlusterServerServiceDao().get(getParameters().getServerServiceId());
Line 65: 
I think this code would need to be written to handle start/stop on one or more 
servers. So serverService id will not be the parameter to look for. I would 
suggest a list of service names as parameter. 

This can support start/stop of one or more services on a server or cluster 
depending on what is set in the passed parameters
Line 66:         if (serverService == null) {
Line 67:             return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_SERVER_SERVICE_NOT_FOUND);
Line 68:         }
Line 69: 


Line 93:     @Override
Line 94:     protected void executeCommand() {
Line 95:         VDSReturnValue returnValue =
Line 96:                 
runVdsCommand(manageActionDetailsMap.get(getParameters().getActionType()).getVdsCmdType(),
Line 97:                         new 
GlusterServiceVDSParameters(serverService.getServerId(), 
getParameters().getActionType(), serverService.getServiceName()));
Maybe you could initialize the ManageActionDetail class in the constructor, 
instead of looking it up in the map each time?
Line 98:         setSucceeded(returnValue.getSucceeded());
Line 99:         if (getSucceeded()) {
Line 100:             
serverService.setStatus(manageActionDetailsMap.get(getParameters().getActionType()).getStatus());
Line 101:             getGlusterServerServiceDao().update(serverService);


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/gluster/GlusterServiceVDSParameters.java
Line 4: import org.ovirt.engine.core.compat.Guid;
Line 5: 
Line 6: public class GlusterServiceVDSParameters extends 
VdsIdVDSCommandParametersBase {
Line 7:     private String actionType;
Line 8:     private String serviceName;
Shouldn't this be an array of serviceNames, since the command accepts an array?
Line 9: 
Line 10:     public GlusterServiceVDSParameters(Guid serverId, String 
actionType, String serviceName) {
Line 11:         super(serverId);
Line 12:         this.actionType = actionType;


....................................................
File 
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
Line 603: GLUSTER_HOOK_CONFLICT_DETECTED=Detected conflict in hook ${HookName} 
of Cluster ${VdsGroupName}.
Line 604: GLUSTER_HOOK_ADDED=Detected new hook ${HookName} in Cluster 
${VdsGroupName}.
Line 605: GLUSTER_HOOK_REMOVED=Detected removal of hook ${HookName} in Cluster 
${VdsGroupName}.
Line 606: GLUSTER_SERVICE_STARTED=Service ${Service} started on server 
${Server}.
Line 607: GLUSTER_SERVICE_START_FAILED=Could not start service ${Service} on 
server ${Server}.  
Please remove trailing whitespace
Line 608: GLUSTER_SERVICE_STOPPED=Service ${service} stopped on Server 
${server}.


....................................................
File 
backend/manager/modules/dal/src/main/resources/bundles/VdsmErrors.properties
Line 351: GlusterHookNotFound=Gluster hook not found
Line 352: GlusterHookListException=Failed to get gluster hook list
Line 353: GlusterHostUUIDNotFound=Gluster host UUID not found
Line 354: GlusterHookConflict=Found conflicting hooks
Line 355: GlusterServicesListFailed=Get service status failed
status or list?
Line 356: GlusterServicesManageFailed=Manage (start/stop/restart) service failed
Line 357: 
Line 358: CANT_RECONSTRUCT_WHEN_A_DOMAIN_IN_POOL_IS_LOCKED=Can't reconstruct 
the Master Domain when the Data Center contains Domains in Locked 
state.\nPlease wait until the operation for these Domains ends before trying to 
reconstruct the Master Domain again.
Line 359: NO_IMPLEMENTATION=Not implemented


--
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to