Ori Liel has posted comments on this change. Change subject: core: Replace FenceExecutor in InitVdsOnUpCommand ......................................................................
Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/38969/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java: Line 188: fence > HostFenceActionExecutor contains two methods: I appreciate documentation, but the method names should still speak for themselves as clearly as possible. Signature #2 means to me intuitively: use agent X to do fence action Y on the host. The reason why this is my intuitive interpretation of this signature, is that the object of all fence operations (START, STOP, RESTART) is always the host, not the agent. Therefore I'd be inclined to think that: fence(FenceActionType.STATUS, agentX) means: use agentX to check the status of the host. But what the above actually does is it checks the status *of the agent*. The method could return "off" while the host itself is actually "on" (because another agent is providing power). For this reason I still feel that providing checkHostStatus() and checkAgentStatus() make for clearer, less error-prone API. -- To view, visit https://gerrit.ovirt.org/38969 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8939f51e8f42f1dc065e74e78b6e9be7011f8082 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Ori Liel <[email protected]> Gerrit-Reviewer: Oved Ourfali <[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
