Oved Ourfali has posted comments on this change. Change subject: core: add support for custom fencing ......................................................................
Patch Set 2: (6 comments) some comments. In addition - do you handle collisions in any way? http://gerrit.ovirt.org/#/c/28621/2/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java: Line 314: config.toString()); Line 315: } Line 316: Line 317: @SuppressWarnings("serial") Line 318: protected <T> T getFenceConfigurationValue(Class<T> clz, ConfigurationValues config, final Version version) { I think this method should be in BackendCapabilitiesResource, and not here. but let's wait for an API maintainer to comment on that. Line 319: return getEntity(clz, Line 320: VdcQueryType.GetFenceConfigurationValue, Line 321: new GetConfigurationValueParameters(config, asString(version)), Line 322: config.toString()); http://gerrit.ovirt.org/#/c/28621/2/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/pm/FenceConfigHelper.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/pm/FenceConfigHelper.java: Line 27: initialized=true; Line 28: } Line 29: Line 30: public static String getCustomKey(String key) { Line 31: return "Custom" + key; Don't think you need that one to be public. You can do this logic here internally. Line 32: } Line 33: Line 34: public static String merge(String key, String value, String customValue) { Line 35: if (!initialized) { Line 30: public static String getCustomKey(String key) { Line 31: return "Custom" + key; Line 32: } Line 33: Line 34: public static String merge(String key, String value, String customValue) { This can be a private method, called from another getFencingConfiguration method that does the merge. In the new method you can add a javadoc saying it will construct the combined system-defined and user-defined configuration. You can add: getSystemDefinedFencingConfiguration getUserDefinedFencingConfiguration and getCombinedFencingConfiguration. although it seems like you only need the last one. Line 35: if (!initialized) { Line 36: init(); Line 37: } Line 38: StringBuilder sb = new StringBuilder(); http://gerrit.ovirt.org/#/c/28621/2/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/pm/VdsFenceOptions.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/pm/VdsFenceOptions.java: Line 77: */ Line 78: private void CacheFencingAgentsOptionMapping() { Line 79: String localFencingOptionMapping = Config.<String> getValue(ConfigValues.VdsFenceOptionMapping, version); Line 80: String customFencingOptionMapping= Config.<String> getValue(ConfigValues.CustomVdsFenceOptionMapping, ConfigCommon.defaultConfigurationVersion); Line 81: localFencingOptionMapping = FenceConfigHelper.merge(ConfigValues.VdsFenceOptionMapping.name(), localFencingOptionMapping, customFencingOptionMapping); So, in this case you'll just call FencingConfigHelper.getFencingConfiguration(ConfigValues.VdsFenceOptionMapping). Line 82: String[] agentsOptionsStr = localFencingOptionMapping.split(Pattern.quote(SEMICOLON), -1); Line 83: for (String agentOptionsStr : agentsOptionsStr) { Line 84: String[] parts = agentOptionsStr.split(Pattern.quote(COLON), -1); Line 85: if (parts.length == 2) { Line 272: * the agent name Line 273: * @return string , the agent real name to be used Line 274: */ Line 275: public static String getRealAgent(String agent) { Line 276: String agentMapping = FenceConfigHelper.merge(ConfigValues.FenceAgentMapping.name(), Config.<String> getValue(ConfigValues.FenceAgentMapping), Config.<String> getValue(ConfigValues.CustomFenceAgentMapping)); same here. Line 277: String realAgent = agent; Line 278: // result has the format [<agent>=<real agent>[,]]* Line 279: String[] settings = agentMapping.split(Pattern.quote(COMMA), -1); Line 280: if (settings.length > 0) { Line 299: * @param fenceOptions Line 300: * @return String the options after adding default agent parameters Line 301: */ Line 302: public static String getDefaultAgentOptions(String agent, String fenceOptions) { Line 303: String agentdefaultParams = FenceConfigHelper.merge(ConfigValues.FenceAgentDefaultParams.name(), Config.<String> getValue(ConfigValues.FenceAgentDefaultParams), Config.<String> getValue(ConfigValues.CustomFenceAgentDefaultParams)); same here. Line 304: StringBuilder realOptions = new StringBuilder(fenceOptions); Line 305: // result has the format [<agent>:param=value[,]...;]* Line 306: String[] params = agentdefaultParams.split(Pattern.quote(SEMICOLON), -1); Line 307: for (String agentOptionsStr : params) { -- To view, visit http://gerrit.ovirt.org/28621 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie887b9025d0813c4d101a886614364127aea75a0 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
