Omer Frenkel has posted comments on this change. Change subject: engine: Phase 3: Configuring ConsoleOptions on backend ......................................................................
Patch Set 7: (6 comments) https://gerrit.ovirt.org/#/c/37974/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ConfigureConsoleOptionsQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ConfigureConsoleOptionsQuery.java: Line 53: Line 54: if (options.getVmId() == null) { Line 55: getQueryReturnValue().setExceptionString("VM id must be filled in console options."); Line 56: return false; Line 57: } maybe check that getCachedVm() != null as well? Line 58: Line 59: return true; Line 60: } Line 61: Line 60: } Line 61: Line 62: @Override Line 63: protected void executeQueryCommand() { Line 64: getQueryReturnValue().setSucceeded(true); afair, this is not needed, queries are succeeded = true by defualt, you only need to set it to false if needed Line 65: ConsoleOptions options = getParameters().getOptions(); Line 66: this.graphicsType = options.getGraphicsType(); Line 67: Line 68: if (!getCachedVm().isRunning()) { Line 103: GraphicsInfo graphicsInfo = getCachedVm().getGraphicsInfos().get(graphicsType); Line 104: Line 105: options.setSmartcardEnabled(getCachedVm().isSmartcardEnabled()); Line 106: options.setNumberOfMonitors(getCachedVm().getNumOfMonitors()); Line 107: options.setGuestHostName(getCachedVm().getVmHost().split("[ ]", -1)[0]); //$NON-NLS-1$ please remove non-nls comment Line 108: if (graphicsInfo.getTlsPort() != null) { Line 109: options.setSecurePort(graphicsInfo.getTlsPort()); Line 110: } Line 111: Line 208: Line 209: return null; Line 210: } Line 211: Line 212: VM getCachedVm() { private? Line 213: if (cachedVm == null) { Line 214: cachedVm = getBackend().runInternalQuery( Line 215: VdcQueryType.GetVmByVmId, Line 216: new IdQueryParameters(getParameters().getOptions().getVmId())).getReturnValue(); Line 212: VM getCachedVm() { Line 213: if (cachedVm == null) { Line 214: cachedVm = getBackend().runInternalQuery( Line 215: VdcQueryType.GetVmByVmId, Line 216: new IdQueryParameters(getParameters().getOptions().getVmId())).getReturnValue(); probably need to pass the filter parameter from the parameters of this class Line 217: } Line 218: Line 219: return cachedVm; Line 220: } https://gerrit.ovirt.org/#/c/37974/7/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/ConsoleOptionsParams.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/ConsoleOptionsParams.java: Line 1: package org.ovirt.engine.core.common.queries; Line 2: Line 3: import org.ovirt.engine.core.common.console.ConsoleOptions; Line 4: Line 5: public class ConsoleOptionsParams extends VdcQueryParametersBase { where is this used? Line 6: Line 7: ConsoleOptions options; Line 8: Line 9: public ConsoleOptionsParams() { } -- To view, visit https://gerrit.ovirt.org/37974 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida16e4de2701d6f6b3f6b3ed52eddca805fa43a7 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[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
