Roy Golan has posted comments on this change. Change subject: core: protocol fall back for older vdsms ......................................................................
Patch Set 2: (4 comments) http://gerrit.ovirt.org/#/c/33728/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/transport/ProtocolDetector.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/transport/ProtocolDetector.java: Line 36: * Attempts to connect to vdsm using a proxy from {@code VdsManager} for a host. Line 37: * Line 38: * @return <code>true</code> if connected or <code>false</code> if connection failed. Line 39: */ Line 40: public boolean attemptConnection() { so if we immediately do a Future.get() then your blocking on it. what's the benefit of using Future? Line 41: try { Line 42: long timeout = Config.<Integer> getValue(ConfigValues.SetupNetworksPollingTimeout); Line 43: FutureVDSCall<VDSReturnValue> task = Line 44: Backend.getInstance().getResourceManager().runFutureVdsCommand(FutureVDSCommandType.TimeBoundPoll, Line 47: task.get(timeout, TimeUnit.SECONDS); Line 48: Line 49: if (returnValue.getSucceeded()) { Line 50: return true; Line 51: } I think Timeout exceptions are converted a return value but I'm not sure. Can you check the proceedProxyReturn() or whatever method is in use there? Line 52: } catch (TimeoutException ignored) { Line 53: } Line 54: return false; Line 55: } Line 80: vdsStatic.setProtocol(VdsProtocol.XML); Line 81: TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { Line 82: @Override Line 83: public Void runInTransaction() { Line 84: DbFacade.getInstance().getVdsStaticDao().update(vdsStatic); is that have to be committed what so ever? Line 85: return null; Line 86: } Line 87: }); Line 88: } http://gerrit.ovirt.org/#/c/33728/2/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/TimeBoundPollVDSCommand.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/TimeBoundPollVDSCommand.java: Line 11: */ Line 12: @Logged(executionLevel = LogLevel.TRACE, errorLevel = LogLevel.DEBUG) Line 13: public class TimeBoundPollVDSCommand<P extends TimeBoundPollVDSCommandParameters> extends FutureVDSCommand<P> { Line 14: Line 15: private long timeout; I guess we can just use the params directly instead local vars? Line 16: private TimeUnit unit; Line 17: Line 18: public TimeBoundPollVDSCommand(P parameters) { Line 19: super(parameters); -- To view, visit http://gerrit.ovirt.org/33728 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie6f48bec60b520c089f326f8c5e79aec288ff3d6 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[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
