From 817ebb6ae66615a4287054e13aa68250502c6f37 Mon Sep 17 00:00:00 2001
From: Gabriel P Santos <gabriel.santos@scclouds.com.br>
Date: Thu, 22 Feb 2024 16:37:14 -0300
Subject: [PATCH 1/3] fix xen nested extraconfig and related exception message

---
 .../xenserver/resource/CitrixResourceBase.java |  2 +-
 .../xenserver/ExtraConfigurationUtility.java   | 18 ++++++++++++------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
index 295c96025ab7..56508b4e2679 100644
--- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
+++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
@@ -1972,7 +1972,7 @@ protected void finalizeVmMetaData(final VM vm, final VM.Record vmr, final Connec
         // Add configuration settings VM record for User VM instances before creating VM
         Map<String, String> extraConfig = vmSpec.getExtraConfig();
         if (vmSpec.getType().equals(VirtualMachine.Type.User) && MapUtils.isNotEmpty(extraConfig)) {
-            logger.info("Appending user extra configuration settings to VM");
+            logger.info(String.format("Appending user extra configuration settings [%s] to [%s].", extraConfig, vmSpec));
             ExtraConfigurationUtility.setExtraConfigurationToVm(conn,vmr, vm, extraConfig);
         }
     }
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 a2702efeb723..736e3880c577 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
@@ -43,6 +43,7 @@ public static void setExtraConfigurationToVm(Connection conn, VM.Record vmr, VM
             String paramValue = configParams.get(paramKey);
 
             //Map params
+            LOGGER.debug(String.format("Applying [%s] configuration as [%s].", paramKey, paramValue));
             if (paramKey.contains(":")) {
                 applyConfigWithNestedKeyValue(conn, vm, recordMap, paramKey, paramValue);
             } else {
@@ -55,6 +56,11 @@ private static boolean isValidOperation(Map<String, Object> recordMap, String ac
         return recordMap.containsKey(actualParam);
     }
 
+    private static Map<String, String> putInMap(Map<String, String> map, String key, String value) {
+        map.put(key, value);
+        return map;
+    }
+
     /**
      * Nested keys contain ":" between the paramKey and need to split into operation param and key
      * */
@@ -71,26 +77,26 @@ private static void applyConfigWithNestedKeyValue(Connection conn, VM vm, Map<St
         try {
             switch (actualParam) {
                 case "VCPUs_params":
-                    vm.addToVCPUsParams(conn, keyName, paramValue);
+                    vm.setVCPUsParams(conn, putInMap(vm.getVCPUsParams(conn), keyName, paramValue));
                     break;
                 case "platform":
-                    vm.addToOtherConfig(conn, keyName, paramValue);
+                    vm.setOtherConfig(conn, putInMap(vm.getOtherConfig(conn), keyName, paramValue));
                     break;
                 case "HVM_boot_params":
-                    vm.addToHVMBootParams(conn, keyName, paramValue);
+                    vm.setHVMBootParams(conn, putInMap(vm.getHVMBootParams(conn), keyName, paramValue));
                     break;
                 case "other_config":
-                    vm.addToOtherConfig(conn, keyName, paramValue);
+                    vm.setOtherConfig(conn, putInMap(vm.getOtherConfig(conn), keyName, paramValue));
                     break;
                 case "xenstore_data":
-                    vm.addToXenstoreData(conn, keyName, paramValue);
+                    vm.setXenstoreData(conn, putInMap(vm.getXenstoreData(conn), keyName, paramValue));
                     break;
                 default:
                     String msg = String.format("Passed configuration %s is not supported", paramKey);
                     LOGGER.warn(msg);
             }
         } catch (XmlRpcException | Types.XenAPIException e) {
-            LOGGER.error("Exception caught while setting VM configuration. exception: " + e.getMessage());
+            LOGGER.error(String.format("Exception caught while setting VM configuration: [%s]", e.getMessage() == null ? e.toString() : e.getMessage()), e);
             throw new CloudRuntimeException("Exception caught while setting VM configuration", e);
         }
     }

From 3f5b434344c3a1b19cd07855108873f7e47223c9 Mon Sep 17 00:00:00 2001
From: Gabriel Pordeus Santos <gabrielpordeus@gmail.com>
Date: Fri, 23 Feb 2024 11:27:16 -0300
Subject: [PATCH 2/3] Apply suggestions from code review

Co-authored-by: dahn <daan.hoogland@gmail.com>
---
 .../hypervisor/xenserver/resource/CitrixResourceBase.java    | 2 +-
 .../hypervisor/xenserver/ExtraConfigurationUtility.java      | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
index 56508b4e2679..a92e0f77b9ab 100644
--- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
+++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
@@ -1972,7 +1972,7 @@ protected void finalizeVmMetaData(final VM vm, final VM.Record vmr, final Connec
         // Add configuration settings VM record for User VM instances before creating VM
         Map<String, String> extraConfig = vmSpec.getExtraConfig();
         if (vmSpec.getType().equals(VirtualMachine.Type.User) && MapUtils.isNotEmpty(extraConfig)) {
-            logger.info(String.format("Appending user extra configuration settings [%s] to [%s].", extraConfig, vmSpec));
+            logger.info("Appending user extra configuration settings [{}] to [{}].", extraConfig, vmSpec));
             ExtraConfigurationUtility.setExtraConfigurationToVm(conn,vmr, vm, extraConfig);
         }
     }
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 736e3880c577..64d83c1427b6 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
@@ -43,7 +43,7 @@ public static void setExtraConfigurationToVm(Connection conn, VM.Record vmr, VM
             String paramValue = configParams.get(paramKey);
 
             //Map params
-            LOGGER.debug(String.format("Applying [%s] configuration as [%s].", paramKey, paramValue));
+            LOGGER.debug("Applying [{}] configuration as [{}].", paramKey, paramValue);
             if (paramKey.contains(":")) {
                 applyConfigWithNestedKeyValue(conn, vm, recordMap, paramKey, paramValue);
             } else {
@@ -96,7 +96,8 @@ private static void applyConfigWithNestedKeyValue(Connection conn, VM vm, Map<St
                     LOGGER.warn(msg);
             }
         } catch (XmlRpcException | Types.XenAPIException e) {
-            LOGGER.error(String.format("Exception caught while setting VM configuration: [%s]", e.getMessage() == null ? e.toString() : e.getMessage()), e);
+            LOGGER.error("Exception caught while setting VM configuration: [{}]", e.getMessage() == null ? e.toString() : e.getMessage());
+            LOGGER.debug("Exception caught while setting VM configuration", e);
             throw new CloudRuntimeException("Exception caught while setting VM configuration", e);
         }
     }

From df0cd8ee84bb3e13b0b39132cbda4bbbd1e693e6 Mon Sep 17 00:00:00 2001
From: Gabriel Pordeus Santos <gabrielpordeus@gmail.com>
Date: Tue, 27 Feb 2024 14:15:43 -0300
Subject: [PATCH 3/3] Update
 plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java

---
 .../cloud/hypervisor/xenserver/resource/CitrixResourceBase.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
index a92e0f77b9ab..8f03648deb6e 100644
--- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
+++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
@@ -1972,7 +1972,7 @@ protected void finalizeVmMetaData(final VM vm, final VM.Record vmr, final Connec
         // Add configuration settings VM record for User VM instances before creating VM
         Map<String, String> extraConfig = vmSpec.getExtraConfig();
         if (vmSpec.getType().equals(VirtualMachine.Type.User) && MapUtils.isNotEmpty(extraConfig)) {
-            logger.info("Appending user extra configuration settings [{}] to [{}].", extraConfig, vmSpec));
+            logger.info("Appending user extra configuration settings [{}] to [{}].", extraConfig, vmSpec);
             ExtraConfigurationUtility.setExtraConfigurationToVm(conn,vmr, vm, extraConfig);
         }
     }