diff --git a/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java b/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java index 46d0a39cf6f..a0a51b121c7 100644 --- a/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java +++ b/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java @@ -16,13 +16,13 @@ // under the License. package org.apache.cloudstack.hypervisor.xenserver; -import java.util.HashMap; import java.util.Map; import org.apache.log4j.Logger; import org.apache.xmlrpc.XmlRpcException; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; import com.xensource.xenapi.Connection; import com.xensource.xenapi.Types; @@ -35,16 +35,22 @@ public class ExtraConfigurationUtility { Map recordMap = vmr.toMap(); for (String key : extraConfig.keySet()) { String cfg = extraConfig.get(key); - Map configParams = prepareKeyValuePair(cfg); + // cfg is either param=value or map-param:key=value + Pair configParam = prepareKeyValuePair(cfg); + if (configParam == null) { + LOG.warn("Invalid extra config passed: " + cfg); + continue; + } - // paramKey is either param or param:key for map parameters - String paramKey = configParams.keySet().toString().replaceAll("[\\[\\]]", ""); - String paramValue = configParams.get(paramKey); + // paramKey is either param or map-param:key for map parameters + String paramKey = configParam.first(); + String paramValue = configParam.second(); - //Map params if (paramKey.contains(":")) { + // Map params - paramKey is map-param:key applyConfigWithNestedKeyValue(conn, vm, recordMap, paramKey, paramValue); } else { + // Params - paramKey is param applyConfigWithKeyValue(conn, vm, recordMap, paramKey, paramValue); } } @@ -58,6 +64,7 @@ public class ExtraConfigurationUtility { * Nested keys contain ":" between the paramKey and need to split into operation param and key * */ private static void applyConfigWithNestedKeyValue(Connection conn, VM vm, Map recordMap, String paramKey, String paramValue) { + // paramKey is map-param:key int i = paramKey.indexOf(":"); String actualParam = paramKey.substring(0, i); String keyName = paramKey.substring(i + 1); @@ -68,12 +75,13 @@ public class ExtraConfigurationUtility { } try { + // map-param param with '_' switch (actualParam) { case "VCPUs_params": vm.addToVCPUsParams(conn, keyName, paramValue); break; case "platform": - vm.addToOtherConfig(conn, keyName, paramValue); + vm.addToPlatform(conn, keyName, paramValue); break; case "HVM_boot_params": vm.addToHVMBootParams(conn, keyName, paramValue); @@ -101,6 +109,7 @@ public class ExtraConfigurationUtility { } try { + // param with '_' switch (paramKey) { case "HVM_boot_policy": vm.setHVMBootPolicy(conn, paramValue); @@ -144,7 +153,7 @@ public class ExtraConfigurationUtility { case "VCPUs_at_startup": vm.setVCPUsAtStartup(conn, Long.valueOf(paramValue)); break; - case "is-a-template": + case "is_a_template": vm.setIsATemplate(conn, Boolean.valueOf(paramValue)); break; case "memory_static_max": @@ -169,12 +178,28 @@ public class ExtraConfigurationUtility { } } - private static Map prepareKeyValuePair(String cfg) { - Map configKeyPair = new HashMap<>(); + protected static Pair prepareKeyValuePair(String cfg) { + // cfg is either param=value or map-param:key=value int indexOfEqualSign = cfg.indexOf("="); - String key = cfg.substring(0, indexOfEqualSign).replace("-", "_"); + if (indexOfEqualSign <= 0) { + return null; + } + + String key; + // Replace '-' with '_' in param / map-param only + if (cfg.contains(":")) { + int indexOfColon = cfg.indexOf(":"); + if (indexOfColon <= 0 || indexOfEqualSign < indexOfColon) { + return null; + } + String mapParam = cfg.substring(0, indexOfColon).replace("-", "_"); + String paramKey = cfg.substring(indexOfColon + 1, indexOfEqualSign); + key = mapParam + ":" + paramKey; + } else { + key = cfg.substring(0, indexOfEqualSign).replace("-", "_"); + } + String value = cfg.substring(indexOfEqualSign + 1); - configKeyPair.put(key, value); - return configKeyPair; + return new Pair<>(key, value); } } diff --git a/plugins/hypervisors/xenserver/src/test/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtilityTest.java b/plugins/hypervisors/xenserver/src/test/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtilityTest.java new file mode 100644 index 00000000000..a1746c23e1b --- /dev/null +++ b/plugins/hypervisors/xenserver/src/test/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtilityTest.java @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.cloudstack.hypervisor.xenserver; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.utils.Pair; + +@RunWith(MockitoJUnitRunner.class) +public class ExtraConfigurationUtilityTest { + + @Test + public void prepareKeyValuePairTest() { + // Map params + verifyKeyValuePairForConfigParam("platform:exp-nested-hvm=true", "platform:exp-nested-hvm", "true"); + verifyKeyValuePairForConfigParam("other_config:my_key=my_value", "other_config:my_key", "my_value"); + verifyKeyValuePairForConfigParam("test-config:test-key=test-value", "test_config:test-key", "test-value"); + + // Params + verifyKeyValuePairForConfigParam("is_a_template=true", "is_a_template", "true"); + verifyKeyValuePairForConfigParam("is-a-template=true", "is_a_template", "true"); + verifyKeyValuePairForConfigParam("memory_dynamic_min=536870912", "memory_dynamic_min", "536870912"); + verifyKeyValuePairForConfigParam("VCPUs_at_startup=2", "VCPUs_at_startup", "2"); + verifyKeyValuePairForConfigParam("VCPUs-max=4", "VCPUs_max", "4"); + } + + private void verifyKeyValuePairForConfigParam(String cfg, String expectedKey, String expectedValue) { + Pair keyValuePair = ExtraConfigurationUtility.prepareKeyValuePair(cfg); + Assert.assertEquals(expectedKey, keyValuePair.first()); + Assert.assertEquals(expectedValue, keyValuePair.second()); + } +}