Martin Peřina has posted comments on this change. Change subject: core: Fix skip fencing if host connected to storage ......................................................................
Patch Set 2: (3 comments) There's nothing new, this patch only adds logic that existed before Ori's patch and unfortunately it was missed during Ori's changes. I'm planning to refactor this fencing result passing during fence flow refactoring, but for now, I'd focus on fixing the "skip fencing if connected to storage" feature http://gerrit.ovirt.org/#/c/36581/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceVdsBaseCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceVdsBaseCommand.java: Line 137: if (wasSkippedDueToPolicy(result)) { Line 138: // when fencing is skipped due to policy we want to suppress command result logging, because Line 139: // we fire an alert in VdsNotRespondingTreatment Line 140: setCommandShouldBeLogged(false); Line 141: setSucceeded(false); > so skip fencing == failure? There's a problem in current result of fence verb, because we can return skipped for two cases: 1) If host is already in desired state, then we return successful operation but with skipped status 2) If host is not fence, because it's still connected to storage, then we return error operation with skipped status Line 142: } else { Line 143: setSucceeded(result.getSucceeded()); Line 144: } Line 145: setActionReturnValue(result); Line 428: Line 429: /** Line 430: * Returns numbers of agents for which fencing operation was skipped Line 431: */ Line 432: protected int countSkippedOperations(List<VDSFenceReturnValue> results) { > If we know the policy, and we checked it, why do we need this logic? This is only for concurrent agents. Prior to Ori's patch we had only primary and secondary agent, now we have a list of agents. Line 433: int numOfSkipped = 0; Line 434: for (VDSFenceReturnValue result : results) { Line 435: if (wasSkippedDueToPolicy(result)) { Line 436: numOfSkipped++; http://gerrit.ovirt.org/#/c/36581/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVdsCommand.java: Line 160: VDSFenceReturnValue result = getMostRelevantResult(results); Line 161: Line 162: if (result != null) { Line 163: if (result.getSucceeded()) { Line 164: int numOfSkipped = countSkippedOperations(results); > why would one agent skip, and not the other? The logic is the same as before. Prior to Ori's patch we had: 1) If primary and secondary skipped => fencing is skipped 2) If primary or secondary skipped => error 3) If primary and seconday didn't skip => continue with fencing Line 165: if (numOfSkipped == 0) { Line 166: // no agent reported that stop operation was skipped, continue with stop operation Line 167: handleSpecificCommandActions(); Line 168: } else { -- To view, visit http://gerrit.ovirt.org/36581 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I84fa56e75adf988ce36b1154269dab8974cf05bf Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
