Liran Zelkha has posted comments on this change. Change subject: core: Remove error messages from log when adding host ......................................................................
Patch Set 1: (9 comments) http://gerrit.ovirt.org/#/c/24980/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsCommand.java: Line 51: protected void initializeVds() { Line 52: initializeVds(true); Line 53: } Line 54: Line 55: protected void initializeVds(boolean expectHostToBeFound) { > I would change the name of the flag to isNewHost and revert the logic in th Done Line 56: Backend.getInstance() Line 57: .getResourceManager() Line 58: .RunVdsCommand(VDSCommandType.RemoveVds, Line 59: new RemoveVdsVDSCommandParameters(getVdsId(), expectHostToBeFound)); http://gerrit.ovirt.org/#/c/24980/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/RemoveVdsVDSCommandParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/RemoveVdsVDSCommandParameters.java: Line 11: Line 12: public RemoveVdsVDSCommandParameters() { Line 13: } Line 14: Line 15: public RemoveVdsVDSCommandParameters(Guid vdsId, boolean errorIfHostDoesntExist) { > errorIfHostDoesntExist => isNewHost Done Line 16: super(vdsId); Line 17: this.errorIfHostDoesntExist = errorIfHostDoesntExist; Line 18: } Line 19: http://gerrit.ovirt.org/#/c/24980/1/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/AddVdsVDSCommand.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/AddVdsVDSCommand.java: Line 7: import org.ovirt.engine.core.utils.log.LogFactory; Line 8: Line 9: public class AddVdsVDSCommand<P extends AddVdsVDSCommandParameters> extends VdsIdVDSCommandBase<P> { Line 10: public AddVdsVDSCommand(P parameters) { Line 11: super(parameters, false); > false=>true (after changing to isNewHost) Done Line 12: } Line 13: Line 14: @Override Line 15: protected void executeVdsIdCommand() { http://gerrit.ovirt.org/#/c/24980/1/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/RemoveVdsVDSCommand.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/RemoveVdsVDSCommand.java: Line 6: Line 7: private P parameters; Line 8: Line 9: public RemoveVdsVDSCommand(P parameters) { Line 10: super(parameters, parameters.isErrorIfHostDoesntExist()); > isErrorIfHostDoesntExist => isNewHost Done Line 11: this.parameters = parameters; Line 12: } Line 13: Line 14: @Override http://gerrit.ovirt.org/#/c/24980/1/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java: Line 223: } Line 224: } Line 225: Line 226: public VdsManager GetVdsManager(Guid vdsId) { Line 227: return GetVdsManager(vdsId, true); > true=>false Done Line 228: } Line 229: Line 230: public VdsManager GetVdsManager(Guid vdsId, boolean errorIfManagerNotFound) { Line 231: VdsManager vdsManger = vdsManagersDict.get(vdsId); Line 229: Line 230: public VdsManager GetVdsManager(Guid vdsId, boolean errorIfManagerNotFound) { Line 231: VdsManager vdsManger = vdsManagersDict.get(vdsId); Line 232: if (vdsManger == null) { Line 233: if (errorIfManagerNotFound) { > if (! isNewHost) Done Line 234: log.errorFormat("Cannot get vdsManager for vdsid={0}", vdsId); Line 235: } else { Line 236: log.infoFormat("Cannot get vdsManager for vdsid={0}", vdsId); Line 237: } Line 232: if (vdsManger == null) { Line 233: if (errorIfManagerNotFound) { Line 234: log.errorFormat("Cannot get vdsManager for vdsid={0}", vdsId); Line 235: } else { Line 236: log.infoFormat("Cannot get vdsManager for vdsid={0}", vdsId); > I don't think we have to log that in this case at all , its a new host .... Done Line 237: } Line 238: } Line 239: return vdsManger; Line 240: } http://gerrit.ovirt.org/#/c/24980/1/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsIdVDSCommandBase.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsIdVDSCommandBase.java: Line 9: protected VdsManager _vdsManager; Line 10: Line 11: public VdsIdVDSCommandBase(P parameters, boolean expectVdsManager) { Line 12: super(parameters); Line 13: _vdsManager = ResourceManager.getInstance().GetVdsManager(parameters.getVdsId(), expectVdsManager); > IIUC the result of that is null if the flag is false , so why not simply wr Done Line 14: } Line 15: Line 16: public VdsIdVDSCommandBase(P parameters) { Line 17: this(parameters, true); Line 13: _vdsManager = ResourceManager.getInstance().GetVdsManager(parameters.getVdsId(), expectVdsManager); Line 14: } Line 15: Line 16: public VdsIdVDSCommandBase(P parameters) { Line 17: this(parameters, true); > true=>false Done Line 18: } Line 19: Line 20: protected Guid getVdsId() { Line 21: return getParameters().getVdsId(); -- To view, visit http://gerrit.ovirt.org/24980 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia17656dbfa0e16457565b04b849eb80ea74a8949 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[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
