Sahina Bose has posted comments on this change. Change subject: engine: Enable gluster hook on cluster ......................................................................
Patch Set 10: (4 inline comments) Kanagaraj, thanks for catching those issues! Will upload another patchset .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterHookCommandBase.java Line 26: protected void updateServerHookStatusInDb(Guid hookId, Guid serverId, GlusterHookStatus status) { Line 27: getGlusterHooksDao().updateGlusterServerHookStatus(hookId, serverId, status); Line 28: } Line 29: Line 30: protected void updateServerHookInDb(GlusterHookEntity hook) { Done Line 31: getGlusterHooksDao().updateGlusterHook(hook); Line 32: } Line 33: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterHookStatusChangeCommand.java Line 50: Line 51: if (Guid.isNullOrEmpty(getParameters().getHookId())) { Line 52: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_GLUSTER_HOOK_ID_IS_REQUIRED); Line 53: return false; Line 54: } Hook already enabled - I think we should still send a request to enable. Because the sync interval of hooks would be set to a high interval and if the hook status is not correctly reflected in db, we cannot rely on that. What I will add is handling of "hook already enabled" vdsm errors. Line 55: Line 56: return true; Line 57: } Line 58: Line 84: Line 85: VDSReturnValue retValue = pairResult.getSecond(); Line 86: if (retValue.getSucceeded()) { Line 87: // update status in database Line 88: updateServerHookStatusInDb(entity.getId(), pairResult.getFirst().getId(), getNewStatus()); We have an entry in server_hooks for all servers in the cluster - so updating if enabled. Line 89: } else { Line 90: errors.add(retValue.getVdsError().getMessage()); Line 91: } Line 92: } Line 100: } Line 101: //The intention was to enable hook. So we update the entity with status enabled irrespective of errors Line 102: entity.setStatus(getNewStatus()); Line 103: updateServerHookInDb(entity); Line 104: Like I said above, there are already entries for all server hooks. So if server is down, will leave status as is. Line 105: } Line 106: Line 107: Line 108: protected abstract VDSCommandType getStatusChangeVDSCommand(); -- To view, visit http://gerrit.ovirt.org/10906 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icc6f9c77393ebed2803ec2a1b295a09f61642c31 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Selvasundaram <sesub...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: Selvasundaram <sesub...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches