Kanagaraj M has posted comments on this change.

Change subject: gluster: Command to update gluster hook on servers
......................................................................


Patch Set 2: (2 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/UpdateGlusterHookCommand.java
Line 64:             if 
(!serverHook.getStatus().equals(GlusterHookStatus.MISSING)) {
Line 65:                 serverHooks.add(serverHook);
Line 66:             }
Line 67:         }
Line 68:         return serverHooks;
There could be a problem when a hook has both content and status conflict but 
in different servers. 

Lets say for a hook 'H1', server1 has content conflict and server2 has status 
conflict. In this case this method will return both the server hook entries and 
then we will update the content in both the servers. 

Though the end result is same(server2 content is same as before), we should try 
to avoid this.

Also the method can be renamed to be more inline with what it does.
Line 69:     }
Line 70: 
Line 71:     @Override
Line 72:     protected boolean canDoAction() {


Line 76: 
Line 77:         if (getMissingServerHooks().isEmpty()) {
Line 78:             
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_GLUSTER_HOOK_NO_CONFLICT_SERVERS);
Line 79:             return false;
Line 80:         }
We should also check the servers which we want to update the hook content is in 
UP state.
Line 81: 
Line 82:         return true;
Line 83:     }
Line 84: 


--
To view, visit http://gerrit.ovirt.org/14935
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibccccbbed1e07d471dc472c41843f46767b9e083
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sahina Bose <[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