Allow additional configuration metadata to VMs (#3510)

* Suqash commits to a single commit and rebase against master

Update marvin tests to use white list

* * Fix marvin test failure
* Add new marvin negative tests cases
* Remove hard-coded hypervisor types in marvin tests

* Fix build error after rebase and add hugepagesless

* Fix readability of python code

* Fix failing test

* Adding cleanup of vms for negative tests

* Bug fixes - change config checks properly and block extraconfig in details

* Trim to compare the keys

* CR comments

* Don't skip extraconfig without exception

Co-authored-by: Boris Stoyanov - a.k.a Bobby <bss.stoyanov@gmail.com>

api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java

@@ -26,8 +26,6 @@ import java.util.Map;
 
 import javax.annotation.Nonnull;
 
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.affinity.AffinityGroupResponse;
 import org.apache.cloudstack.api.ACL;
@@ -52,6 +50,7 @@ import org.apache.cloudstack.api.response.UserVmResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.commons.collections.MapUtils;
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.api.LogLevel;
 import com.cloud.event.EventTypes;
@@ -190,7 +189,7 @@ public class DeployVMCmd extends BaseAsyncCreateCustomIdCmd implements SecurityG
     @Parameter(name = ApiConstants.DISPLAY_VM, type = CommandType.BOOLEAN, since = "4.2", description = "an optional field, whether to the display the vm to the end user or not.", authorized = {RoleType.Admin})
     private Boolean displayVm;
 
-    @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, since = "4.3", description = "used to specify the custom parameters.")
+    @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, since = "4.3", description = "used to specify the custom parameters. 'extraconfig' is not allowed to be passed in details")
     private Map details;
 
     @Parameter(name = ApiConstants.DEPLOYMENT_PLANNER, type = CommandType.STRING, description = "Deployment planner to use for vm allocation. Available to ROOT admin only", since = "4.4", authorized = { RoleType.Admin })

api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java

@@ -98,7 +98,7 @@ public class UpdateVMCmd extends BaseCustomIdCmd implements SecurityGroupAction,
     @Parameter(name = ApiConstants.INSTANCE_NAME, type = CommandType.STRING, description = "instance name of the user vm", since = "4.4", authorized = {RoleType.Admin})
     private String instanceName;
 
-    @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "Details in key/value pairs.")
+    @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "Details in key/value pairs. 'extraconfig' is not allowed to be passed in details.")
     protected Map<String, String> details;
 
     @ACL

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -39,11 +39,8 @@ import java.util.concurrent.TimeUnit;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import com.cloud.agent.api.PrepareForMigrationAnswer;
-import com.cloud.agent.api.to.DpdkTO;
-import com.cloud.offering.NetworkOffering;
-import com.cloud.offerings.dao.NetworkOfferingDetailsDao;
 import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
+import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.command.admin.vm.MigrateVMCmd;
 import org.apache.cloudstack.api.command.admin.volume.MigrateVolumeCmdByAdmin;
 import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd;
@@ -97,6 +94,7 @@ import com.cloud.agent.api.ModifyTargetsCommand;
 import com.cloud.agent.api.PingRoutingCommand;
 import com.cloud.agent.api.PlugNicAnswer;
 import com.cloud.agent.api.PlugNicCommand;
+import com.cloud.agent.api.PrepareForMigrationAnswer;
 import com.cloud.agent.api.PrepareForMigrationCommand;
 import com.cloud.agent.api.RebootAnswer;
 import com.cloud.agent.api.RebootCommand;
@@ -116,6 +114,7 @@ import com.cloud.agent.api.UnPlugNicCommand;
 import com.cloud.agent.api.UnregisterVMCommand;
 import com.cloud.agent.api.routing.NetworkElementCommand;
 import com.cloud.agent.api.to.DiskTO;
+import com.cloud.agent.api.to.DpdkTO;
 import com.cloud.agent.api.to.GPUDeviceTO;
 import com.cloud.agent.api.to.NicTO;
 import com.cloud.agent.api.to.VirtualMachineTO;
@@ -166,7 +165,9 @@ import com.cloud.network.dao.NetworkVO;
 import com.cloud.network.router.VirtualRouter;
 import com.cloud.offering.DiskOffering;
 import com.cloud.offering.DiskOfferingInfo;
+import com.cloud.offering.NetworkOffering;
 import com.cloud.offering.ServiceOffering;
+import com.cloud.offerings.dao.NetworkOfferingDetailsDao;
 import com.cloud.org.Cluster;
 import com.cloud.resource.ResourceManager;
 import com.cloud.resource.ResourceState;
@@ -1123,6 +1124,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
 
                     vmGuru.finalizeDeployment(cmds, vmProfile, dest, ctx);
 
+                    // Get VM extraConfig from DB and set to VM TO
+                    addExtraConfig(vmTO);
+
                     work = _workDao.findById(work.getId());
                     if (work == null || work.getStep() != Step.Prepare) {
                         throw new ConcurrentOperationException("Work steps have been changed: " + work);
@@ -1287,6 +1291,16 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
         }
     }
 
+    // Add extra config data to the vmTO as a Map
+    private void addExtraConfig(VirtualMachineTO vmTO) {
+        Map<String, String> details = vmTO.getDetails();
+        for (String key : details.keySet()) {
+            if (key.startsWith(ApiConstants.EXTRA_CONFIG)) {
+                vmTO.addExtraConfig(key, details.get(key));
+            }
+        }
+    }
+
     // for managed storage on KVM, need to make sure the path field of the volume in question is populated with the IQN
     private void handlePath(final DiskTO[] disks, final HypervisorType hypervisorType) {
         if (hypervisorType != HypervisorType.KVM) {

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

@@ -2236,7 +2236,11 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
 
         vm.addComp(devices);
 
+        // Add extra configuration to User VM Domain XML before starting
+        if (vmTO.getType().equals(VirtualMachine.Type.User) && MapUtils.isNotEmpty(extraConfig)) {
+            s_logger.info("Appending extra configuration data to guest VM domain XML");
             addExtraConfigComponent(extraConfig, vm);
+        }
 
         return vm;
     }

plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java

@@ -49,6 +49,7 @@ import javax.naming.ConfigurationException;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 
+import org.apache.cloudstack.hypervisor.xenserver.ExtraConfigurationUtility;
 import org.apache.cloudstack.storage.to.TemplateObjectTO;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
 import org.apache.commons.collections.CollectionUtils;
@@ -1404,7 +1405,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
             }
         }
         try {
-            finalizeVmMetaData(vm, conn, vmSpec);
+            finalizeVmMetaData(vm, vmr, conn, vmSpec);
         } catch (final Exception e) {
             throw new CloudRuntimeException("Unable to finalize VM MetaData: " + vmSpec);
         }
@@ -1859,7 +1860,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
         }
     }
 
-    protected void finalizeVmMetaData(final VM vm, final Connection conn, final VirtualMachineTO vmSpec) throws Exception {
+    protected void finalizeVmMetaData(final VM vm, final VM.Record vmr, final Connection conn, final VirtualMachineTO vmSpec) throws Exception {
 
         final Map<String, String> details = vmSpec.getDetails();
         if (details != null) {
@@ -1890,6 +1891,13 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
                 }
             }
         }
+
+        // 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)) {
+            s_logger.info("Appending user extra configuration settings to VM");
+            ExtraConfigurationUtility.setExtraConfigurationToVm(conn,vmr, vm, extraConfig);
+        }
     }
 
     /**

plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java

@@ -0,0 +1,180 @@
+// 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 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.exception.CloudRuntimeException;
+import com.xensource.xenapi.Connection;
+import com.xensource.xenapi.Types;
+import com.xensource.xenapi.VM;
+
+public class ExtraConfigurationUtility {
+    private static final Logger LOG = Logger.getLogger(ExtraConfigurationUtility.class);
+
+    public static void setExtraConfigurationToVm(Connection conn, VM.Record vmr, VM vm, Map<String, String> extraConfig) {
+        Map<String, Object> recordMap = vmr.toMap();
+        for (String key : extraConfig.keySet()) {
+            String cfg = extraConfig.get(key);
+            Map<String, String> configParams = prepareKeyValuePair(cfg);
+
+            // paramKey is either param or param:key for map parameters
+            String paramKey = configParams.keySet().toString().replaceAll("[\\[\\]]", "");
+            String paramValue = configParams.get(paramKey);
+
+            //Map params
+            if (paramKey.contains(":")) {
+                applyConfigWithNestedKeyValue(conn, vm, recordMap, paramKey, paramValue);
+            } else {
+                applyConfigWithKeyValue(conn, vm, recordMap, paramKey, paramValue);
+            }
+        }
+    }
+
+    private static boolean isValidOperation(Map<String, Object> recordMap, String actualParam) {
+        return recordMap.containsKey(actualParam);
+    }
+
+    /**
+     * Nested keys contain ":" between the paramKey and need to split into operation param and key
+     * */
+    private static void applyConfigWithNestedKeyValue(Connection conn, VM vm, Map<String, Object> recordMap, String paramKey, String paramValue) {
+        int i = paramKey.indexOf(":");
+        String actualParam = paramKey.substring(0, i);
+        String keyName = paramKey.substring(i + 1);
+
+        if (!isValidOperation(recordMap, actualParam)) {
+            LOG.error("Unsupported extra configuration has been passed " + actualParam);
+            throw new InvalidParameterValueException("Unsupported extra configuration option has been passed: " + actualParam);
+        }
+
+        try {
+            switch (actualParam) {
+                case "VCPUs_params":
+                    vm.addToVCPUsParams(conn, keyName, paramValue);
+                    break;
+                case "platform":
+                    vm.addToOtherConfig(conn, keyName, paramValue);
+                    break;
+                case "HVM_boot_params":
+                    vm.addToHVMBootParams(conn, keyName, paramValue);
+                    break;
+                case "other_config":
+                    vm.addToOtherConfig(conn, keyName, paramValue);
+                    break;
+                case "xenstore_data":
+                    vm.addToXenstoreData(conn, keyName, paramValue);
+                    break;
+                default:
+                    String msg = String.format("Passed configuration %s is not supported", paramKey);
+                    LOG.warn(msg);
+            }
+        } catch (XmlRpcException | Types.XenAPIException e) {
+            LOG.error("Exception caught while setting VM configuration. exception: " + e.getMessage());
+            throw new CloudRuntimeException("Exception caught while setting VM configuration", e);
+        }
+    }
+
+    private static void applyConfigWithKeyValue(Connection conn, VM vm, Map<String, Object> recordMap, String paramKey, String paramValue) {
+        if (!isValidOperation(recordMap, paramKey)) {
+            LOG.error("Unsupported extra configuration has been passed: " + paramKey);
+            throw new InvalidParameterValueException("Unsupported extra configuration parameter key has been passed: " + paramKey);
+        }
+
+        try {
+            switch (paramKey) {
+                case "HVM_boot_policy":
+                    vm.setHVMBootPolicy(conn, paramValue);
+                    break;
+                case "HVM_shadow_multiplier":
+                    vm.setHVMShadowMultiplier(conn, Double.valueOf(paramValue));
+                    break;
+                case "PV_kernel":
+                    vm.setPVKernel(conn, paramValue);
+                    break;
+                case "PV_ramdisk":
+                    vm.setPVRamdisk(conn, paramValue);
+                    break;
+                case "PV_args":
+                    vm.setPVArgs(conn, paramValue);
+                    break;
+                case "PV_legacy_args":
+                    vm.setPVLegacyArgs(conn, paramValue);
+                    break;
+                case "PV_bootloader":
+                    vm.setPVBootloader(conn, paramValue);
+                    break;
+                case "PV_bootloader_args":
+                    vm.setPVBootloaderArgs(conn, paramValue);
+                    break;
+                case "ha_restart_priority":
+                    vm.setHaRestartPriority(conn, paramValue);
+                    break;
+                case "start_delay":
+                    vm.setStartDelay(conn, Long.valueOf(paramValue));
+                    break;
+                case "shutdown_delay":
+                    vm.setShutdownDelay(conn, Long.valueOf(paramValue));
+                    break;
+                case "order":
+                    vm.setOrder(conn, Long.valueOf(paramValue));
+                    break;
+                case "VCPUs_max":
+                    vm.setVCPUsMax(conn, Long.valueOf(paramValue));
+                    break;
+                case "VCPUs_at_startup":
+                    vm.setVCPUsAtStartup(conn, Long.valueOf(paramValue));
+                    break;
+                case "is-a-template":
+                    vm.setIsATemplate(conn, Boolean.valueOf(paramValue));
+                    break;
+                case "memory_static_max":
+                    vm.setMemoryStaticMax(conn, Long.valueOf(paramValue));
+                    break;
+                case "memory_static_min":
+                    vm.setMemoryStaticMin(conn, Long.valueOf(paramValue));
+                    break;
+                case "memory_dynamic_max":
+                    vm.setMemoryDynamicMax(conn, Long.valueOf(paramValue));
+                    break;
+                case "memory_dynamic_min":
+                    vm.setMemoryDynamicMin(conn, Long.valueOf(paramValue));
+                    break;
+                default:
+                    String anotherMessage = String.format("Passed configuration %s is not supported", paramKey);
+                    LOG.error(anotherMessage);
+            }
+        } catch (XmlRpcException | Types.XenAPIException e) {
+            LOG.error("Exception caught while setting VM configuration, exception: " + e.getMessage());
+            throw new CloudRuntimeException("Exception caught while setting VM configuration: ", e);
+        }
+    }
+
+    private static Map<String, String> prepareKeyValuePair(String cfg) {
+        Map<String, String> configKeyPair = new HashMap<>();
+        int indexOfEqualSign = cfg.indexOf("=");
+        String key = cfg.substring(0, indexOfEqualSign).replace("-", "_");
+        String value = cfg.substring(indexOfEqualSign + 1);
+        configKeyPair.put(key, value);
+        return configKeyPair;
+    }
+}

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -16,6 +16,8 @@
 // under the License.
 package com.cloud.vm;
 
+import java.io.IOException;
+import java.io.StringReader;
 import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;
 import java.util.ArrayList;
@@ -34,13 +36,17 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
 
-import com.cloud.storage.ScopeType;
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.affinity.AffinityGroupService;
@@ -99,6 +105,11 @@ import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.NodeList;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.agent.api.Answer;
@@ -234,6 +245,7 @@ import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.DiskOfferingVO;
 import com.cloud.storage.GuestOSCategoryVO;
 import com.cloud.storage.GuestOSVO;
+import com.cloud.storage.ScopeType;
 import com.cloud.storage.Snapshot;
 import com.cloud.storage.SnapshotVO;
 import com.cloud.storage.Storage;
@@ -265,7 +277,6 @@ import com.cloud.template.VirtualMachineTemplate;
 import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
 import com.cloud.user.AccountService;
-import com.cloud.user.AccountVO;
 import com.cloud.user.ResourceLimitService;
 import com.cloud.user.SSHKeyPair;
 import com.cloud.user.SSHKeyPairVO;
@@ -530,8 +541,18 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
     private static final ConfigKey<Boolean> AllowDeployVmIfGivenHostFails = new ConfigKey<Boolean>("Advanced", Boolean.class, "allow.deploy.vm.if.deploy.on.given.host.fails", "false",
             "allow vm to deploy on different host if vm fails to deploy on the given host ", true);
 
-    private static final ConfigKey<Boolean> EnableAdditionalVmConfig = new ConfigKey<>("Advanced", Boolean.class, "enable.additional.vm.configuration",
+    private static final ConfigKey<Boolean> EnableAdditionalVmConfig = new ConfigKey<>("Advanced", Boolean.class,
-            "false", "allow additional arbitrary configuration to vm", true, ConfigKey.Scope.Account);
+            "enable.additional.vm.configuration", "false", "allow additional arbitrary configuration to vm", true, ConfigKey.Scope.Account);
+
+    private static final ConfigKey<String> KvmAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class,
+            "allow.additional.vm.configuration.list.kvm", "", "Comma separated list of allowed additional configuration options.", true);
+
+    private static final ConfigKey<String> XenServerAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class,
+            "allow.additional.vm.configuration.list.xenserver", "", "Comma separated list of allowed additional configuration options", true);
+
+    private static final ConfigKey<String> VmwareAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class,
+            "allow.additional.vm.configuration.list.vmware", "", "Comma separated list of allowed additional configuration options.", true);
+
     private static final ConfigKey<Boolean> VmDestroyForcestop = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.destroy.forcestop", "false",
             "On destroy, force-stop takes this value ", true);
 
@@ -2479,6 +2500,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             }
         } else {
             if (MapUtils.isNotEmpty(details)) {
+                if (details.containsKey("extraconfig")) {
+                    throw new InvalidParameterValueException("'extraconfig' should not be included in details as key");
+                }
+
                 if (caller != null && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) {
                     // Ensure blacklisted detail is not passed by non-root-admin user
                     for (final String detailName : details.keySet()) {
@@ -2505,9 +2530,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
                 vmInstance.setDetails(details);
                 _vmDao.saveDetails(vmInstance);
             }
-            if (StringUtils.isNotBlank(extraConfig) && EnableAdditionalVmConfig.valueIn(accountId)) {
+            if (StringUtils.isNotBlank(extraConfig)) {
-                AccountVO account = _accountDao.findById(accountId);
+                if (EnableAdditionalVmConfig.valueIn(accountId)) {
-                addExtraConfig(vmInstance, account, extraConfig);
+                    s_logger.info("Adding extra configuration to user vm: " + vmInstance.getUuid());
+                    addExtraConfig(vmInstance, extraConfig);
+                } else {
+                    throw new InvalidParameterValueException("attempted setting extraconfig but enable.additional.vm.configuration is disabled");
+                }
             }
         }
         return updateVirtualMachine(id, displayName, group, ha, isDisplayVm, osTypeId, userData, isDynamicallyScalable,
@@ -5086,8 +5115,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
         Account caller = CallContext.current().getCallingAccount();
         Long callerId = caller.getId();
         String extraConfig = cmd.getExtraConfig();
-        if (StringUtils.isNotBlank(extraConfig) && EnableAdditionalVmConfig.valueIn(callerId) ) {
+        if (StringUtils.isNotBlank(extraConfig)) {
-            addExtraConfig(vm, caller, extraConfig);
+            if (EnableAdditionalVmConfig.valueIn(callerId)) {
+                s_logger.info("Adding extra configuration to user vm: " + vm.getUuid());
+                addExtraConfig(vm, extraConfig);
+            } else {
+                throw new InvalidParameterValueException("attempted setting extraconfig but enable.additional.vm.configuration is disabled");
+            }
         }
 
         if (cmd.getCopyImageTags()) {
@@ -5106,24 +5140,119 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
     }
 
     /**
-     * Persist extra configurations as details for VMware VMs
+     * Persist extra configuration data in the user_vm_details table as key/value pair
+     * @param decodedUrl String consisting of the extra config data to appended onto the vmx file for VMware instances
      */
     protected void persistExtraConfigVmware(String decodedUrl, UserVm vm) {
-        String[] configDataArr = decodedUrl.split("\\r?\\n");
+        boolean isValidConfig = isValidKeyValuePair(decodedUrl);
-        for (String config: configDataArr) {
+        if (isValidConfig) {
-            String[] keyValue = config.split("=");
+            String[] extraConfigs = decodedUrl.split("\\r?\\n");
-            try {
+            for (String cfg : extraConfigs) {
-                userVmDetailsDao.addDetail(vm.getId(), keyValue[0], keyValue[1], true);
+                // Validate cfg against unsupported operations set by admin here
-            } catch (ArrayIndexOutOfBoundsException e) {
+                String[] allowedKeyList = VmwareAdditionalConfigAllowList.value().split(",");
-                throw new CloudRuntimeException("Issue occurred during parsing of:" + config);
+                boolean validXenOrVmwareConfiguration = isValidXenOrVmwareConfiguration(cfg, allowedKeyList);
+                String[] paramArray = cfg.split("=");
+                if (validXenOrVmwareConfiguration && paramArray.length == 2) {
+                    userVmDetailsDao.addDetail(vm.getId(), paramArray[0].trim(), paramArray[1].trim(), true);
+                } else {
+                    throw new CloudRuntimeException("Extra config " + cfg + " is not on the list of allowed keys for VMware hypervisor hosts.");
                 }
             }
+        } else {
+            throw new CloudRuntimeException("The passed extra config string " + decodedUrl + "contains an invalid key/value pair pattern");
+        }
     }
 
     /**
-     * Persist extra configurations as details for hypervisors except Vmware
+     * Used to persist extra configuration settings in user_vm_details table for the XenServer hypervisor
+     * persists config as key/value pair e.g key = extraconfig-1 , value="PV-bootloader=pygrub" and so on to extraconfig-N where
+     * N denotes the number of extra configuration settings passed by user
+     *
+     * @param decodedUrl A string containing extra configuration settings as key/value pairs seprated by newline escape character
+     *                   e.x PV-bootloader=pygrub\nPV-args=console\nHV-Boot-policy=""
      */
-    protected void persistExtraConfigNonVmware(String decodedUrl, UserVm vm) {
+    protected void persistExtraConfigXenServer(String decodedUrl, UserVm vm) {
+        boolean isValidConfig = isValidKeyValuePair(decodedUrl);
+        if (isValidConfig) {
+            String[] extraConfigs = decodedUrl.split("\\r?\\n");
+            int i = 1;
+            String extraConfigKey = ApiConstants.EXTRA_CONFIG + "-";
+            for (String cfg : extraConfigs) {
+                // Validate cfg against unsupported operations set by admin here
+                String[] allowedKeyList = XenServerAdditionalConfigAllowList.value().split(",");
+                boolean validXenOrVmwareConfiguration = isValidXenOrVmwareConfiguration(cfg, allowedKeyList);
+                if (validXenOrVmwareConfiguration) {
+                    userVmDetailsDao.addDetail(vm.getId(), extraConfigKey + String.valueOf(i), cfg, true);
+                    i++;
+                } else {
+                    throw new CloudRuntimeException("Extra config " + cfg + " is not on the list of allowed keys for XenServer hypervisor hosts.");
+                }
+            }
+        } else {
+            String msg = String.format("The passed extra config string '%s' contains an invalid key/value pair pattern", decodedUrl);
+            throw new CloudRuntimeException(msg);
+        }
+    }
+
+    /**
+     * Used to valid extraconfig keylvalue pair for Vmware and XenServer
+     * Example of tested valid config for VMware as taken from VM instance vmx file
+     * <p>
+     * nvp.vm-uuid=34b3d5ea-1c25-4bb0-9250-8dc3388bfa9b
+     * migrate.hostLog=i-2-67-VM-5130f8ab.hlog
+     * ethernet0.address=02:00:5f:51:00:41
+     * </p>
+     * <p>
+     * Examples of tested valid configs for XenServer
+     * <p>
+     * is-a-template=true\nHVM-boot-policy=\nPV-bootloader=pygrub\nPV-args=hvc0
+     * </p>
+     *
+     * Allow the following character set {', ", -, ., =, a-z, 0-9, empty space, \n}
+     *
+     * @param decodedUrl String conprising of extra config key/value pairs for XenServer and Vmware
+     * @return True if extraconfig is valid key/value pair
+     */
+    protected boolean isValidKeyValuePair(String decodedUrl) {
+        // Valid pairs should look like "key-1=value1, param:key-2=value2, my.config.v0=False"
+        Pattern pattern = Pattern.compile("^(?:[\\w-\\s\\.:]*=[\\w-\\s\\.'\":]*(?:\\s+|$))+$");
+        Matcher matcher = pattern.matcher(decodedUrl);
+        return matcher.matches();
+    }
+
+    /**
+     * Validates key/value pair strings passed as extra configuration for XenServer and Vmware
+     * @param cfg configuration key-value pair
+     * @param allowedKeyList list of allowed configuration keys for XenServer and VMware
+     * @return
+     */
+    protected boolean isValidXenOrVmwareConfiguration(String cfg, String[] allowedKeyList) {
+        // This should be of minimum length 1
+        // Value is ignored in case it is empty
+        String[] cfgKeyValuePair = cfg.split("=");
+        if (cfgKeyValuePair.length >= 1) {
+            for (String allowedKey : allowedKeyList) {
+                if (cfgKeyValuePair[0].equalsIgnoreCase(allowedKey.trim())) {
+                    return true;
+                }
+            }
+        } else {
+            String msg = String.format("An incorrect configuration %s has been passed", cfg);
+            throw new CloudRuntimeException(msg);
+        }
+        return false;
+    }
+
+    /**
+     * Persist extra configuration data on KVM
+     * persisted in the user_vm_details DB as extraconfig-1, and so on depending on the number of configurations
+     * For KVM, extra config is passed as XML
+     * @param decodedUrl string containing xml configuration to be persisted into user_vm_details table
+     * @param vm
+     */
+    protected void persistExtraConfigKvm(String decodedUrl, UserVm vm) {
+        // validate config against blacklisted cfg commands
+        validateKvmExtraConfig(decodedUrl);
         String[] extraConfigs = decodedUrl.split("\n\n");
         for (String cfg : extraConfigs) {
             int i = 1;
@@ -5142,16 +5271,71 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
         }
     }
 
-    protected void addExtraConfig(UserVm vm, Account caller, String extraConfig) {
+    /**
-        String decodedUrl = decodeExtraConfig(extraConfig);
+     * This method is called by the persistExtraConfigKvm
-        HypervisorType hypervisorType = vm.getHypervisorType();
+     * Validates passed extra configuration data for KVM and validates against blacklist of unwanted commands
-        if (hypervisorType == HypervisorType.VMware) {
+     * controlled by Root admin
-            persistExtraConfigVmware(decodedUrl, vm);
+     * @param decodedUrl string containing xml configuration to be validated
-        } else {
+     */
-            persistExtraConfigNonVmware(decodedUrl, vm);
+    protected void validateKvmExtraConfig(String decodedUrl) {
+        String[] allowedConfigOptionList = KvmAdditionalConfigAllowList.value().split(",");
+        // Skip allowed keys validation validation for DPDK
+        if (!decodedUrl.contains(":")) {
+            try {
+                DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
+                InputSource src = new InputSource();
+                src.setCharacterStream(new StringReader(String.format("<config>\n%s\n</config>", decodedUrl)));
+                Document doc = builder.parse(src);
+                doc.getDocumentElement().normalize();
+                NodeList nodeList=doc.getElementsByTagName("*");
+                for (int i = 1; i < nodeList.getLength(); i++) { // First element is config so skip it
+                    Element element = (Element)nodeList.item(i);
+                    boolean isValidConfig = false;
+                    String currentConfig = element.getNodeName().trim();
+                    for (String tag : allowedConfigOptionList) {
+                        if (currentConfig.equals(tag.trim())) {
+                            isValidConfig = true;
+                        }
+                    }
+                    if (!isValidConfig) {
+                        throw new CloudRuntimeException(String.format("Extra config %s is not on the list of allowed keys for KVM hypervisor hosts", currentConfig));
+                    }
+                }
+            } catch (ParserConfigurationException | IOException | SAXException e) {
+                throw new CloudRuntimeException("Failed to parse additional XML configuration: " + e.getMessage());
+            }
         }
     }
 
+    /**
+     * Adds extra config data to guest VM instances
+     * @param extraConfig Extra Configuration settings to be added in UserVm instances for KVM, XenServer and VMware
+     */
+    protected void addExtraConfig(UserVm vm, String extraConfig) {
+        String decodedUrl = decodeExtraConfig(extraConfig);
+        HypervisorType hypervisorType = vm.getHypervisorType();
+
+        switch (hypervisorType) {
+            case XenServer:
+                persistExtraConfigXenServer(decodedUrl, vm);
+                break;
+            case KVM:
+                persistExtraConfigKvm(decodedUrl, vm);
+                break;
+            case VMware:
+                persistExtraConfigVmware(decodedUrl, vm);
+                break;
+            default:
+                String msg = String.format("This hypervisor %s is not supported for use with this feature", hypervisorType.toString());
+                throw new CloudRuntimeException(msg);
+        }
+    }
+
+    /**
+     * Decodes an URL encoded string passed as extra configuration for guest VMs
+     * @param encodeString URL encoded string
+     * @return String result of decoded URL
+     */
     protected String decodeExtraConfig(String encodeString) {
         String decodedUrl;
         try {
@@ -5198,6 +5382,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             maxIops = details.get("maxIopsDo");
 
             verifyMinAndMaxIops(minIops, maxIops);
+
+            if (details.containsKey("extraconfig")) {
+                throw new InvalidParameterValueException("'extraconfig' should not be included in details as key");
+            }
         }
     }
 
@@ -6732,8 +6920,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, VmIpFetchThreadPoolMax,
+        return new ConfigKey<?>[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax,
-            VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties};
+                VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties,
+                KvmAdditionalConfigAllowList, XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList};
     }
 
     @Override

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

@@ -16,6 +16,10 @@
 // under the License.
 package com.cloud.vm;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 import java.util.ArrayList;
 import java.util.HashMap;
 
@@ -23,7 +27,6 @@ import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
 import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd;
 import org.apache.cloudstack.context.CallContext;
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -291,6 +294,18 @@ public class UserVmManagerImplTest {
         String returnedMacAddress = userVmManagerImpl.validateOrReplaceMacAddress(macAddress, 1l);
 
         Mockito.verify(networkModel, Mockito.times(times)).getNextAvailableMacAddressInNetwork(Mockito.anyLong());
-        Assert.assertEquals(expectedMacAddress, returnedMacAddress);
+        assertEquals(expectedMacAddress, returnedMacAddress);
+    }
+
+    @Test
+    public void testValidatekeyValuePair() throws Exception {
+        assertTrue(userVmManagerImpl.isValidKeyValuePair("is-a-template=true\nHVM-boot-policy=\nPV-bootloader=pygrub\nPV-args=hvc0"));
+        assertTrue(userVmManagerImpl.isValidKeyValuePair("is-a-template=true HVM-boot-policy= PV-bootloader=pygrub PV-args=hvc0"));
+        assertTrue(userVmManagerImpl.isValidKeyValuePair("nvp.vm-uuid=34b3d5ea-1c25-4bb0-9250-8dc3388bfa9b"));
+        assertFalse(userVmManagerImpl.isValidKeyValuePair("key"));
+        //key-1=value1, param:key-2=value2, my.config.v0=False"
+        assertTrue(userVmManagerImpl.isValidKeyValuePair("key-1=value1"));
+        assertTrue(userVmManagerImpl.isValidKeyValuePair("param:key-2=value2"));
+        assertTrue(userVmManagerImpl.isValidKeyValuePair("my.config.v0=False"));
     }
 }

test/integration/smoke/test_deploy_vm_extra_config_data.py

@@ -0,0 +1,542 @@
+# 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.
+""" BVT tests for Virtual Machine additional configuration
+"""
+# Import System modules
+import urllib
+import xml.etree.ElementTree as ET
+
+from lxml import etree
+from marvin.cloudstackAPI import (updateVirtualMachine,
+                                  deployVirtualMachine,
+                                  destroyVirtualMachine,
+                                  stopVirtualMachine,
+                                  startVirtualMachine,
+                                  updateConfiguration,
+                                  listVirtualMachines)
+# Import Local Modules
+from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.lib.base import (Account,
+                             ServiceOffering,
+                             )
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               list_hosts)
+from marvin.lib.utils import *
+from nose.plugins.attrib import attr
+
+class TestAddConfigtoDeployVM(cloudstackTestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        testClient = super(TestAddConfigtoDeployVM, cls).getClsTestClient()
+        cls.apiclient = testClient.getApiClient()
+        cls.services = testClient.getParsedTestDataConfig()
+
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests())
+        cls.hypervisor = testClient.getHypervisorInfo()
+        cls.services['mode'] = cls.zone.networktype
+        cls.hostConfig = cls.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][
+            0].__dict__
+
+        # Set Zones and disk offerings
+        cls.services["small"]["zoneid"] = cls.zone.id
+
+        cls.services["iso1"]["zoneid"] = cls.zone.id
+
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create an account, network, and IP addresses
+        cls.account = Account.create(
+            cls.apiclient,
+            cls.services["account"],
+            domainid=cls.domain.id
+        )
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offerings"]["small"]
+        )
+
+        cls.cleanup = [
+            cls.account,
+            cls.service_offering
+        ]
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cls.apiclient = super(TestAddConfigtoDeployVM, cls).getClsTestClient().getApiClient()
+            # Clean up, terminate the created templates
+            cleanup_resources(cls.apiclient, cls.cleanup)
+
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+
+    def setUp(self):
+        self.apiclient = self.testClient.getApiClient()
+        self.hypervisor = self.testClient.getHypervisorInfo()
+        self.dbclient = self.testClient.getDbConnection()
+
+        """
+        Set EnableAdditionalData to true
+        """
+        updateConfigurationCmd = updateConfiguration.updateConfigurationCmd()
+        updateConfigurationCmd.name = "enable.additional.vm.configuration"
+        updateConfigurationCmd.value = "true"
+        updateConfigurationCmd.scopename = "account"
+        updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd)
+        self.debug("updated the parameter %s with value %s" % (
+            updateConfigurationResponse.name, updateConfigurationResponse.value))
+
+    # Ste Global Config value
+    def add_global_config(self, name, value):
+        self.apiclient = self.testClient.getApiClient()
+        self.hypervisor = self.testClient.getHypervisorInfo()
+        self.dbclient = self.testClient.getDbConnection()
+
+        cmd = updateConfiguration.updateConfigurationCmd()
+        cmd.name = name
+        cmd.value = value
+        return self.apiclient.updateConfiguration(cmd)
+
+    # Compare XML Element objects
+    def elements_equal(self, e1, e2):
+        if e1.tag != e2.tag:
+            return False
+        if e1.attrib != e2.attrib:
+            return False
+        if len(e1) != len(e2):
+            return False
+        return all(self.elements_equal(c1, c2) for c1, c2 in zip(e1, e2))
+
+    def destroy_vm(self, vm_id):
+        cmd = destroyVirtualMachine.destroyVirtualMachineCmd()
+        cmd.expunge = True
+        cmd.id = vm_id
+        return self.apiclient.destroyVirtualMachine(cmd)
+
+    def deploy_vm(self, hypervisor, extra_config=None):
+        cmd = deployVirtualMachine.deployVirtualMachineCmd()
+        if extra_config is not None:
+            cmd.extraconfig = extra_config
+
+        template = get_template(
+            self.apiclient,
+            self.zone.id,
+            hypervisor=hypervisor
+        )
+        cmd.zoneid = self.zone.id
+        cmd.templateid = template.id
+        cmd.serviceofferingid = self.service_offering.id
+        return self.apiclient.deployVirtualMachine(cmd)
+
+    def list_vm(self):
+        cmd = listVirtualMachines.listVirtualMachinesCmd()
+        cmd.hypervisor = self.hypervisor
+        return self.apiclient.listVirtualMachines(cmd)[0]
+
+    def update_vm(self, id, extra_config):
+        cmd = updateVirtualMachine.updateVirtualMachineCmd()
+        cmd.id = id
+        cmd.extraconfig = extra_config
+        return self.apiclient.updateVirtualMachine(cmd)
+
+    def stop_vm(self, id):
+        cmd = stopVirtualMachine.stopVirtualMachineCmd()
+        cmd.id = id
+        return self.apiclient.stopVirtualMachine(cmd)
+
+    def start_vm(self, id):
+        cmd = startVirtualMachine.startVirtualMachineCmd()
+        cmd.id = id
+        return self.apiclient.startVirtualMachine(cmd)
+
+    # Parse extraconfig for config with that returned by xe vm-param-get ...
+    def get_xen_param_values(self, config):
+        equal_sign_index = config.index("=")
+        cmd_option = config[:equal_sign_index]
+        cmd_value = config[equal_sign_index + 1:]
+        return cmd_option, cmd_value
+
+    # Format vm config such that it equals the one from vmx file
+    def prepare_vmware_config(self, config):
+        equal_sign_index = config.index("=")
+        cmd_option = config[:equal_sign_index]
+        cmd_value = config[equal_sign_index + 1:]
+        return cmd_option + ' = '  '"{}"'.format(cmd_value)
+
+    # Get vm uuid from xenserver host
+    def get_vm_uuid(self, instance_name, ssh_client):
+        cmd = 'xe vm-list name-label={} params=uuid '.format(instance_name)
+        result = ssh_client.execute(cmd)
+        uuid_str = result[0]
+        i = uuid_str.index(":")
+        return uuid_str[i + 1:].strip()
+
+    @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true")
+    def test_01_deploy_vm_with_extraconfig_throws_exception_kvm(self):
+        '''
+        Test that extra config is not added when element tag is not added on the allowed list global config on KVM hosts
+        '''
+
+        hypervisor = self.hypervisor.lower()
+        if hypervisor != 'kvm':
+            raise self.skipTest("Skipping test case for non-kvm hypervisor")
+
+        '''
+        The following extraconfig is required for enabling hugepages on kvm
+        <memoryBacking>
+            <hugepages/>
+        </memoryBacking>
+        url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E'
+        '''
+        extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E"
+
+        try:
+            # Clear KVM allow list to show that code throws exception when command is not included in the list
+            name = 'allow.additional.vm.configuration.list.kvm'
+
+            self.add_global_config(name, "")
+            self.assertRaises(Exception,
+                              self.deploy_vm(hypervisor, extraconfig),
+                              "Exception was not thrown, check kvm global configuration")
+        except Exception as e:
+            logging.debug(e)
+        finally:
+            self.destroy_vm(self.list_vm().id)
+
+    @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true")
+    def test_02_deploy_vm_with_extraconfig_kvm(self):
+        '''
+        Test that extra config is added on KVM hosts
+        '''
+
+        hypervisor = self.hypervisor.lower()
+        if hypervisor != 'kvm':
+            raise self.skipTest("Skipping test case for non-kvm hypervisor")
+
+        name = 'allow.additional.vm.configuration.list.kvm'
+        value = 'memoryBacking, hugepages, unsedConfigKey'
+
+        add_config_response = self.add_global_config(name, value)
+
+        if add_config_response.name:
+            try:
+                '''
+                The following extraconfig is required for enabling hugepages on kvm
+                <memoryBacking>
+                    <hugepages/>
+                </memoryBacking>
+                url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E'
+                '''
+                extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E"
+
+                response = self.deploy_vm(hypervisor, extraconfig)
+
+                host_id = response.hostid
+                host = list_hosts(
+                    self.apiclient,
+                    id=host_id,
+                    hypervisor=hypervisor)
+
+                instance_name = response.instancename
+                host_ipaddress = host[0].ipaddress
+
+                ssh_client = SshClient(host_ipaddress, port=22,
+                                       user=self.hostConfig['username'],
+                                       passwd=self.hostConfig['password'])
+                virsh_cmd = 'virsh dumpxml %s' % instance_name
+                xml_res = ssh_client.execute(virsh_cmd)
+                xml_as_str = ''.join(xml_res)
+
+                extraconfig_decoded_xml = '<config>' + urllib.unquote(extraconfig) + '</config>'
+
+                # Root XML Elements
+                parser = etree.XMLParser(remove_blank_text=True)
+                domain_xml_root = ET.fromstring(xml_as_str, parser=parser)
+                decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser)
+                for child in decoded_xml_root:
+                    find_element_in_domain_xml = domain_xml_root.find(child.tag)
+
+                    # Fail if extra config is not found in domain xml
+                    self.assertNotEquals(
+                        0,
+                        len(find_element_in_domain_xml),
+                        'Element tag from extra config not added to VM'
+                    )
+
+                    # Compare found XML node with extra config node
+                    is_a_match = self.elements_equal(child, find_element_in_domain_xml)
+                    self.assertEquals(
+                        True,
+                        is_a_match,
+                        'The element from tags from extra config do not match with those found in domain xml'
+                    )
+            finally:
+                self.destroy_vm(response.id)
+                self.add_global_config(name, "")
+
+    @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true")
+    def test_03_update_vm_with_extraconfig_kvm(self):
+        '''
+        Test that extra config is added on KVM hosts
+        '''
+
+        hypervisor = self.hypervisor.lower()
+        if hypervisor != 'kvm':
+            raise self.skipTest("Skipping test case for non-kvm hypervisor")
+
+        name = 'allow.additional.vm.configuration.list.kvm'
+        value = 'memoryBacking, hugepages'
+
+        add_config_response = self.add_global_config(name, value)
+
+        if add_config_response.name:
+            try:
+                '''
+                The following extraconfig is required for enabling hugepages on kvm
+                <memoryBacking>
+                    <hugepages/>
+                </memoryBacking>
+                url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E'
+                '''
+                extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E"
+
+                response = self.deploy_vm(hypervisor)
+                vm_id = response.id
+
+                '''
+                For updateVirtualMachineCmd, the VM must be stopped and restarted for changes to take effect
+                '''
+                self.stop_vm(vm_id)
+                self.update_vm(vm_id, extraconfig)
+                start_resp = self.start_vm(vm_id)
+
+                host_id = start_resp.hostid
+                host = list_hosts(
+                    self.apiclient,
+                    id=host_id,
+                    hypervisor=hypervisor)
+
+                instance_name = response.instancename
+                host_ipaddress = host[0].ipaddress
+
+                ssh_client = SshClient(host_ipaddress, port=22,
+                                       user=self.hostConfig['username'],
+                                       passwd=self.hostConfig['password'])
+                virsh_cmd = 'virsh dumpxml %s' % instance_name
+                xml_res = ssh_client.execute(virsh_cmd)
+                xml_as_str = ''.join(xml_res)
+
+                extraconfig_decoded_xml = '<config>' + urllib.unquote(extraconfig) + '</config>'
+
+                # Root XML Elements
+                parser = etree.XMLParser(remove_blank_text=True)
+                domain_xml_root = ET.fromstring(xml_as_str, parser=parser)
+                decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser)
+                for child in decoded_xml_root:
+                    find_element_in_domain_xml = domain_xml_root.find(child.tag)
+
+                    # Fail if extra config is not found in domain xml
+                    self.assertNotEquals(
+                        0,
+                        len(find_element_in_domain_xml),
+                        'Element tag from extra config not added to VM'
+                    )
+
+                    # Compare found XML node with extra config node
+                    is_a_match = self.elements_equal(child, find_element_in_domain_xml)
+                    self.assertEquals(
+                        True,
+                        is_a_match,
+                        'The element from tags from extra config do not match with those found in domain xml'
+                    )
+            finally:
+                self.destroy_vm(vm_id)
+                self.add_global_config(name, "")
+
+    @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true")
+    def test_04_deploy_vm_with_extraconfig_throws_exception_vmware(self):
+        '''
+        Test that extra config is not added when configuration key is not added on the allowed list global config for VMWARE hosts
+        '''
+
+        hypervisor = self.hypervisor.lower()
+        if hypervisor != 'vmware':
+            raise self.skipTest("Skipping test case for non-vmware hypervisor")
+
+        '''
+        The following extra configuration is used to set Hyper-V instance to run on ESXi host
+        hypervisor.cpuid.v0 = FALSE
+        '''
+        extraconfig = 'hypervisor.cpuid.v0%3DFALSE'
+
+        try:
+            # Clear VMWARE allow list to show that code throws exception when command is not included in the list
+            name = 'allow.additional.vm.configuration.list.vmware'
+
+            self.add_global_config(name, "")
+            self.assertRaises(Exception,
+                              self.deploy_vm(hypervisor, extraconfig),
+                              "Exception was not thrown, check VMWARE global configuration")
+        except Exception as e:
+            logging.debug(e)
+        finally:
+            self.destroy_vm(self.list_vm().id)
+
+    @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true")
+    def test_05_deploy_vm_with_extraconfig_vmware(self):
+        '''
+        Test that extra config is added on VMware hosts
+        '''
+        hypervisor = self.hypervisor.lower()
+        if hypervisor != 'vmware':
+            raise self.skipTest("Skipping test case for non-vmware hypervisor")
+
+        name = 'allow.additional.vm.configuration.list.vmware'
+        value = 'hypervisor.cpuid.v0'
+
+        add_config_response = self.add_global_config(name, value)
+
+        if add_config_response.name:
+
+            '''
+            The following extra configuration is used to set Hyper-V instance to run on ESXi host
+            hypervisor.cpuid.v0 = FALSE
+            '''
+            extraconfig = 'hypervisor.cpuid.v0%3DFALSE'
+            try:
+                response = self.deploy_vm(hypervisor, extraconfig)
+                host_id = response.hostid
+                host = list_hosts(
+                    self.apiclient,
+                    id=host_id)
+
+                instance_name = response.instancename
+                host_ipaddress = host[0].ipaddress
+
+                ssh_client = SshClient(host_ipaddress, port=22,
+                                       user=self.hostConfig['username'],
+                                       passwd=self.hostConfig['password'])
+
+                extraconfig_decoded = urllib.unquote(extraconfig)
+                config_arr = extraconfig_decoded.splitlines()
+
+                for config in config_arr:
+                    vmx_config = self.prepare_vmware_config(config)
+                    vmx_file_name = "\"$(esxcli vm process list | grep %s | tail -1 | awk '{print $3}')\"" % instance_name
+                    # parse vm instance vmx file to see if extraconfig has been added
+                    grep_config = "cat %s | grep -w '%s'" % (vmx_file_name, vmx_config)
+                    result = ssh_client.execute(grep_config)
+                    # Match exact configuration from vmx file, return empty result array if configuration is not found
+                    self.assertNotEquals(
+                        0,
+                        len(result),
+                        'Extra  configuration not found in instance vmx file'
+                    )
+            finally:
+                self.destroy_vm(response.id)
+                self.add_global_config(name, "")
+
+    @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true")
+    def test_06_deploy_vm_with_extraconfig_throws_exception_xenserver(self):
+        '''
+        Test that extra config is not added when configuration key is not added on the allowed list global config for XenServer hosts
+        '''
+
+        hypervisor = self.hypervisor.lower()
+        if hypervisor != 'xenserver':
+            raise self.skipTest("Skipping test case for non-xenserver hypervisor")
+
+        '''
+        Following commands are used to convert a VM from HVM to PV and set using vm-param-set
+        HVM-boot-policy=
+        PV-bootloader=pygrub
+        PV-args=hvc0
+        '''
+
+        extraconfig = 'HVM-boot-policy%3D%0APV-bootloader%3Dpygrub%0APV-args%3Dhvc0'
+
+        try:
+            # Clear VMWARE allow list to show that code throws exception when command is not included in the list
+            name = 'allow.additional.vm.configuration.list.xenserver'
+
+            self.add_global_config(name, "")
+            self.assertRaises(Exception,
+                              self.deploy_vm(hypervisor, extraconfig),
+                              "Exception was not thrown, check XenServer global configuration")
+
+        except Exception as e:
+            logging.debug(e)
+        finally:
+            self.destroy_vm(self.list_vm().id)
+
+    @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true")
+    def test_07_deploy_vm_with_extraconfig_xenserver(self):
+        hypervisor = self.hypervisor.lower()
+        if hypervisor != 'xenserver':
+            raise self.skipTest("Skipping test case for non-xenserver hypervisor")
+        """
+        Following commands are used to convert a VM from HVM to PV and set using vm-param-set
+        HVM-boot-policy=
+        PV-bootloader=pygrub
+        PV-args=hvc0
+        """
+
+        name = 'allow.additional.vm.configuration.list.xenserver'
+        value = 'HVM-boot-policy, PV-bootloader, PV-args'
+
+        add_config_response = self.add_global_config(name, value)
+
+        if add_config_response.name:
+            extraconfig = 'HVM-boot-policy%3D%0APV-bootloader%3Dpygrub%0APV-args%3Dhvc0'
+            try:
+                response = self.deploy_vm(hypervisor, extraconfig)
+                host_id = response.hostid
+                host = list_hosts(
+                    self.apiclient,
+                    id=host_id)
+
+                host_ipaddress = host[0].ipaddress
+
+                ssh_client = SshClient(host_ipaddress, port=22,
+                                       user=self.hostConfig['username'],
+                                       passwd=self.hostConfig['password'])
+
+                extraconfig_decoded = urllib.unquote(extraconfig)
+                config_arr = extraconfig_decoded.splitlines()
+
+                # Get vm instance uuid
+                instance_uuid = self.get_vm_uuid(response.instancename, ssh_client)
+                for config in config_arr:
+                    config_tuple = self.get_xen_param_values(config)
+                    # Log on to XenServer host and check the vm-param-get
+                    vm_config_check = 'xe vm-param-get param-name={} uuid={}'.format(config_tuple[0], instance_uuid)
+                    result = ssh_client.execute(vm_config_check)
+                    param_value = config_tuple[1].strip()
+                    # Check if each configuration command has set the configuration as sent with extraconfig
+                    self.assertEquals(
+                        param_value,
+                        result[0],
+                        'Extra  configuration not found in VM param list'
+                    )
+            finally:
+                self.destroy_vm(response.id)
+                self.add_global_config(name, "")