Oved Ourfali has posted comments on this change. Change subject: core: Add kdump detection into fencing flow ......................................................................
Patch Set 3: (7 comments) http://gerrit.ovirt.org/#/c/28072/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsKdumpDetectionCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsKdumpDetectionCommand.java: Line 47: /** Line 48: * If the VDS is not responding, it tries to detect if VDS is kdumping or not. Line 49: */ Line 50: @Override Line 51: protected void executeCommand() { I'd create some functions here, rather than putting it all in executeCommand. For your consideration. Line 52: setVds(null); Line 53: if (getVds() == null) { Line 54: setCommandShouldBeLogged(false); Line 55: log.infoFormat("Kdump detection will not be executed on host {0}({1}) since it doesn't exist anymore.", Line 59: return; Line 60: } Line 61: Line 62: // check if fence_kdump listener is alive Line 63: checkListenerAlive(); If it isn't alive should we continue? Line 64: Line 65: boolean detected = false; Line 66: int interval = Config.<Integer>getValue(ConfigValues.FenceKdumpMessageInterval) * 1000; Line 67: Calendar cal = Calendar.getInstance(); Line 66: int interval = Config.<Integer>getValue(ConfigValues.FenceKdumpMessageInterval) * 1000; Line 67: Calendar cal = Calendar.getInstance(); Line 68: cal.add(Calendar.SECOND, Config.<Integer>getValue(ConfigValues.KdumpStartedTimeout)); Line 69: long timeout = cal.getTimeInMillis(); Line 70: VdsKdumpStatus kdumpStatus = getDbFacade().getVdsKdumpStatusDao().get(getVdsId()); Can't you initialize that one in the loop? Line 71: Line 72: while (!detected && timeout > System.currentTimeMillis()) { Line 73: detected = kdumpStatus != null; Line 74: if (!detected) { Line 88: getReturnValue().setSucceeded(false); Line 89: return; Line 90: } Line 91: Line 92: // kdump detected set status to reboot and wait until kdump fisnihes typo in the work finishes. I'd write "until kdump is done" Line 93: Backend.getInstance() Line 94: .getResourceManager() Line 95: .RunVdsCommand(VDSCommandType.SetVdsStatus, Line 96: new SetVdsStatusVDSCommandParameters(getVdsId(), VDSStatus.Reboot)); Line 92: // kdump detected set status to reboot and wait until kdump fisnihes Line 93: Backend.getInstance() Line 94: .getResourceManager() Line 95: .RunVdsCommand(VDSCommandType.SetVdsStatus, Line 96: new SetVdsStatusVDSCommandParameters(getVdsId(), VDSStatus.Reboot)); So we're setting it to reboot before it really happened? Assuming it will after the kdump is done? Line 97: Line 98: while (kdumpStatus.getStatus() != KdumpFlowStatus.FINISHED) { Line 99: ThreadUtils.sleep(interval); Line 100: kdumpStatus = getDbFacade().getVdsKdumpStatusDao().get(getVdsId()); Line 111: Line 112: getReturnValue().setSucceeded(true); Line 113: } Line 114: Line 115: private boolean isListenerTimeouted(Date lastHeartbeat) { s/Timeouted/TimedOut Line 116: long timeout = lastHeartbeat.getTime() Line 117: + Config.<Integer>getValue(ConfigValues.FenceKdumpListenerTimeout) * 1000; Line 118: return System.currentTimeMillis() > timeout; Line 119: } Line 119: } Line 120: Line 121: private void checkListenerAlive() { Line 122: ExternalVariable fkAlive = getDbFacade().getExternalVariableDao().get(LISTENER_HEARTBEAT); Line 123: if (fkAlive == null || isListenerTimeouted(fkAlive.getUpdated())) { s/Timeouted/timedOut Line 124: AuditLogableBase base = new AuditLogableBase(); Line 125: base.setVds(getVds()); Line 126: AuditLogDirector.log(base, AuditLogType.FENCE_KDUMP_LISTENER_IS_NOT_ALIVE); Line 127: } -- To view, visit http://gerrit.ovirt.org/28072 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I18145bc5814dc0b97b8ed6593f0c76473285ae16 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Martin Peřina <[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
