From 61ab95d4bbf7466fd2e49228ed6929c9d3f02f7a Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 13 Jun 2024 17:44:35 +0530 Subject: [PATCH 1/4] Update extraconfig for platform param in xen/xcpng --- .../hypervisor/xenserver/ExtraConfigurationUtility.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 46d0a39cf6f1..a4c78e72dc02 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 @@ -73,7 +73,7 @@ private static void applyConfigWithNestedKeyValue(Connection conn, VM vm, Map Date: Fri, 14 Jun 2024 09:25:52 +0530 Subject: [PATCH 2/4] Fix map param key, not to replace '-' with '_' (replace only applicable to param / map-param) --- .../xenserver/ExtraConfigurationUtility.java | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) 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 a4c78e72dc02..a828e53d5e89 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 static void setExtraConfigurationToVm(Connection conn, VM.Record vmr, VM 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 @@ private static boolean isValidOperation(Map recordMap, String ac * 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,6 +75,7 @@ private static void applyConfigWithNestedKeyValue(Connection conn, VM vm, Map prepareKeyValuePair(String cfg) { - Map configKeyPair = new HashMap<>(); + private 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); } } From b0ef6b99eb8ce47982b718e0b6580ba88fab8952 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 14 Jun 2024 20:18:33 +0530 Subject: [PATCH 3/4] Added unit tests --- .../xenserver/ExtraConfigurationUtility.java | 2 +- .../ExtraConfigurationUtilityTest.java | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 plugins/hypervisors/xenserver/src/test/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtilityTest.java 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 a828e53d5e89..a0a51b121c7a 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 @@ -178,7 +178,7 @@ private static void applyConfigWithKeyValue(Connection conn, VM vm, Map prepareKeyValuePair(String cfg) { + protected static Pair prepareKeyValuePair(String cfg) { // cfg is either param=value or map-param:key=value int indexOfEqualSign = cfg.indexOf("="); if (indexOfEqualSign <= 0) { 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 000000000000..c5dd9c53b145 --- /dev/null +++ b/plugins/hypervisors/xenserver/src/test/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtilityTest.java @@ -0,0 +1,33 @@ +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()); + } +} From bc8ff74df561194b822b6272a65d8a40583f01ac Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Sat, 15 Jun 2024 19:30:17 +0530 Subject: [PATCH 4/4] Add license for tests file --- .../ExtraConfigurationUtilityTest.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) 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 index c5dd9c53b145..a1746c23e1b6 100644 --- 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 @@ -1,3 +1,22 @@ +/* + * 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;