Ori Liel has posted comments on this change. Change subject: core: Fix skip fencing if host connected to storage ......................................................................
Patch Set 2: (3 comments) 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 207: fenceAgent.getId()); Line 208: } else if (wasSkippedDueToPolicy(fenceExecutionResult)) { Line 209: // fencing execution was skipped due to fencing policy Line 210: return fenceExecutionResult; Line 211: } else { > This is not a bug, we need to distinguish between two skipping statuses: I didn't mean that what you did here is a bug, I meant the opposite, what you did here does indeed fix a bug... Line 212: if (fenceExecutionResult.getSucceeded()) { Line 213: boolean requiredStatusAchieved = waitForStatus(); Line 214: int i = 0; Line 215: while (!requiredStatusAchieved && i < retries) { http://gerrit.ovirt.org/#/c/36581/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsCommand.java: Line 51: public class RestartVdsCommand<T extends FenceVdsActionParameters> extends VdsCommand<T> { Line 52: Line 53: private static final String INTERNAL_FENCE_USER = "Engine"; Line 54: Line 55: protected boolean skippedDueToFencingPolicy; > 1. VdsNotRespondingTreatmentCommand.executeCommand() calls super.executeCo yup, missed the call to 'super()', thanks for the clarification Line 56: Line 57: /** Line 58: * Constructor for command creation when compensation is applied on startup Line 59: * Line 211: if (fenceReturnValue.getReturnValue() instanceof FenceStatusReturnValue) { Line 212: skipped = ((FenceStatusReturnValue) fenceReturnValue.getReturnValue()).getIsSkipped(); Line 213: } Line 214: } Line 215: return skipped; > I don't see any bug here. You're misunderstanding me, I'm not saying that you inserted a bug, but that you fixed one. It's like saying nice catch, there is indeed a bug in the code, and it is fixed by your change. Line 216: } Line 217: Line 218: @Override Line 219: public String getUserName() { -- 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 <[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: [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
