From 3bb318bab905d974f4613c637dee1d6e5d7ecb32 Mon Sep 17 00:00:00 2001 From: Bryan Lima <42067040+BryanMLima@users.noreply.github.com> Date: Wed, 13 Dec 2023 02:21:24 -0300 Subject: [PATCH] kvm: Add support for cgroupv2 (#8252) 1. Problem description In Apache CloudStack (ACS), when a VM is deployed in a host with the KVM hypervisor, an XML file is created in the assigned host, which has a property shares that defines the weight of the VM to access the host CPU. The value of this property has no unit, and it is a relative measure to calculate how much CPU a given VM will have in the host. However, this value has a limit, which depends on the version of cgroup utilized by the host's kernel. The problem lies at the range value of shares that varies between both versions: [2, 264144] for cgroups version 1; and [1, 10000] for cgroups version 2. Currently, ACS calculates the value of shares using Equation 1, presented below, where CPU is the number of cores and speed is the CPU frequency; both specified in the VM's compute offering. Therefore, if a compute offering has, for example, 6 cores at 2 GHz, the shares value will be 12000 and an exception will be thrown by libvirt if the host utilizes cgroup v2. The second version is becoming the default one in current Linux distributions; thus, it is necessary to address this limitation. Equation 1 shares = CPU * speed Fixes: #6744 2. Proposed changes To address the problem described, we propose to apply a scale conversion considering the max shares of the host. Using the same formula currently utilized by ACS, it is possible to calculate the maximum shares of a VM for a given host. In other words, using the number of cores and the nominal speed of the host's CPU as the upper limit of shares allowed to a VM. Then, this value will be scaled to the allowed interval of [1, 10000] of cgroup v2 by using a linear scale conversion. The VM shares would be calculated as Equation 2, presented below, where VM requested shares is the requested shares value calculated using Equation 1, cgroup upper limit is fixed with a value of 10000 (cgroups v2 upper limit), and host max shares is the maximum shares value of the host, calculated using Equation 1. Using Equation 2, the only case where a VM passes the cgroup v2 limit is when the user requests more resources than the host has, which is not possible with the current implementation of ACS. Equation 2 shares = (VM requested shares * cgroup upper limit)/host max shares To implement the proposal, the following APIs will be updated: deployVirtualMachine, migrateVirtualMachine and scaleVirtualMachine. When a VM is being deployed, a new verification will be added to find a suitable host. The max shares of each host will be calculated, and the VM calculated shares will be verified if it does not surpass the host's value. Likewise, the migration of VMs will have a similar new verification. Lastly, the scale of VMs will also have the same verification for the VM's host. To determine the max shares of a given host, we will use the same equation currently used in ACS for calculating the shares of VMs, presented in Section 1. When Equation 1 is used to determine the maximum shares of a host, CPU is the number of cores of the host, and speed is the nominal CPU speed, i.e., considering the CPU's base frequency. It is important to note that these changes are only for hosts with the KVM hypervisor using cgroup v2 for now. --- .../com/cloud/agent/api/MigrateCommand.java | 11 +++ .../agent/api/PrepareForMigrationAnswer.java | 10 ++ .../cloud/vm/VirtualMachineManagerImpl.java | 67 +++++++------ .../StorageSystemDataMotionStrategy.java | 13 ++- .../resource/LibvirtComputingResource.java | 82 +++++++++++++++- .../wrapper/LibvirtMigrateCommandWrapper.java | 41 ++++++++ ...virtPrepareForMigrationCommandWrapper.java | 22 ++++- .../wrapper/LibvirtScaleVmCommandWrapper.java | 3 +- .../LibvirtComputingResourceTest.java | 93 +++++++++++++++++++ .../LibvirtMigrateCommandWrapperTest.java | 80 ++++++++++++++++ ...PrepareForMigrationCommandWrapperTest.java | 75 +++++++++++++++ .../LibvirtScaleVmCommandWrapperTest.java | 5 + 12 files changed, 463 insertions(+), 39 deletions(-) create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapperTest.java diff --git a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java index 27251f4bb78..3acdb9c351b 100644 --- a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java +++ b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java @@ -40,6 +40,9 @@ public class MigrateCommand extends Command { private boolean executeInSequence = false; private List migrateDiskInfoList = new ArrayList<>(); private Map dpdkInterfaceMapping = new HashMap<>(); + + private int newVmCpuShares; + Map vlanToPersistenceMap = new HashMap<>(); public Map getDpdkInterfaceMapping() { @@ -138,6 +141,14 @@ public class MigrateCommand extends Command { this.migrateDiskInfoList = migrateDiskInfoList; } + public int getNewVmCpuShares() { + return newVmCpuShares; + } + + public void setNewVmCpuShares(int newVmCpuShares) { + this.newVmCpuShares = newVmCpuShares; + } + public static class MigrateDiskInfo { public enum DiskType { FILE, BLOCK; diff --git a/core/src/main/java/com/cloud/agent/api/PrepareForMigrationAnswer.java b/core/src/main/java/com/cloud/agent/api/PrepareForMigrationAnswer.java index d0a544ba081..190e844ddc5 100644 --- a/core/src/main/java/com/cloud/agent/api/PrepareForMigrationAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/PrepareForMigrationAnswer.java @@ -28,6 +28,8 @@ public class PrepareForMigrationAnswer extends Answer { private Map dpdkInterfaceMapping = new HashMap<>(); + private Integer newVmCpuShares = null; + protected PrepareForMigrationAnswer() { } @@ -50,4 +52,12 @@ public class PrepareForMigrationAnswer extends Answer { public Map getDpdkInterfaceMapping() { return this.dpdkInterfaceMapping; } + + public Integer getNewVmCpuShares() { + return newVmCpuShares; + } + + public void setNewVmCpuShares(Integer newVmCpuShares) { + this.newVmCpuShares = newVmCpuShares; + } } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 7792afd2c63..4c8883476a2 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -48,6 +48,7 @@ import javax.naming.ConfigurationException; import javax.persistence.EntityExistsException; import com.cloud.event.ActionEventUtils; +import com.google.gson.Gson; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -2790,23 +2791,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } boolean migrated = false; - Map dpdkInterfaceMapping = null; + Map dpdkInterfaceMapping = new HashMap<>(); try { - final boolean isWindows = _guestOsCategoryDao.findById(_guestOsDao.findById(vm.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows"); - Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); - final MigrateCommand mc = new MigrateCommand(vm.getInstanceName(), dest.getHost().getPrivateIpAddress(), isWindows, to, getExecuteInSequence(vm.getHypervisorType())); - if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { - mc.setVlanToPersistenceMap(vlanToPersistenceMap); - } - - boolean kvmAutoConvergence = StorageManager.KvmAutoConvergence.value(); - mc.setAutoConvergence(kvmAutoConvergence); - mc.setHostGuid(dest.getHost().getGuid()); - - dpdkInterfaceMapping = ((PrepareForMigrationAnswer) pfma).getDpdkInterfaceMapping(); - if (MapUtils.isNotEmpty(dpdkInterfaceMapping)) { - mc.setDpdkInterfaceMapping(dpdkInterfaceMapping); - } + final MigrateCommand mc = buildMigrateCommand(vm, to, dest, pfma, dpdkInterfaceMapping); try { final Answer ma = _agentMgr.send(vm.getLastHostId(), mc); @@ -2878,6 +2865,43 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } + /** + * Create and set parameters for the {@link MigrateCommand} used in the migration and scaling of VMs. + */ + protected MigrateCommand buildMigrateCommand(VMInstanceVO vmInstance, VirtualMachineTO virtualMachineTO, DeployDestination destination, Answer answer, + Map dpdkInterfaceMapping) { + final boolean isWindows = _guestOsCategoryDao.findById(_guestOsDao.findById(vmInstance.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows"); + final MigrateCommand migrateCommand = new MigrateCommand(vmInstance.getInstanceName(), destination.getHost().getPrivateIpAddress(), isWindows, virtualMachineTO, + getExecuteInSequence(vmInstance.getHypervisorType())); + + Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vmInstance.getId()); + if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { + s_logger.debug(String.format("Setting VLAN persistence to [%s] as part of migrate command for VM [%s].", new Gson().toJson(vlanToPersistenceMap), virtualMachineTO)); + migrateCommand.setVlanToPersistenceMap(vlanToPersistenceMap); + } + + migrateCommand.setAutoConvergence(StorageManager.KvmAutoConvergence.value()); + migrateCommand.setHostGuid(destination.getHost().getGuid()); + + PrepareForMigrationAnswer prepareForMigrationAnswer = (PrepareForMigrationAnswer) answer; + + Map answerDpdkInterfaceMapping = prepareForMigrationAnswer.getDpdkInterfaceMapping(); + if (MapUtils.isNotEmpty(answerDpdkInterfaceMapping) && dpdkInterfaceMapping != null) { + s_logger.debug(String.format("Setting DPDK interface mapping to [%s] as part of migrate command for VM [%s].", new Gson().toJson(vlanToPersistenceMap), + virtualMachineTO)); + dpdkInterfaceMapping.putAll(answerDpdkInterfaceMapping); + migrateCommand.setDpdkInterfaceMapping(dpdkInterfaceMapping); + } + + Integer newVmCpuShares = prepareForMigrationAnswer.getNewVmCpuShares(); + if (newVmCpuShares != null) { + s_logger.debug(String.format("Setting CPU shares to [%d] as part of migrate command for VM [%s].", newVmCpuShares, virtualMachineTO)); + migrateCommand.setNewVmCpuShares(newVmCpuShares); + } + + return migrateCommand; + } + private void updateVmPod(VMInstanceVO vm, long dstHostId) { // update the VMs pod HostVO host = _hostDao.findById(dstHostId); @@ -4395,16 +4419,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac boolean migrated = false; try { - Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); - final boolean isWindows = _guestOsCategoryDao.findById(_guestOsDao.findById(vm.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows"); - final MigrateCommand mc = new MigrateCommand(vm.getInstanceName(), dest.getHost().getPrivateIpAddress(), isWindows, to, getExecuteInSequence(vm.getHypervisorType())); - if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { - mc.setVlanToPersistenceMap(vlanToPersistenceMap); - } - - boolean kvmAutoConvergence = StorageManager.KvmAutoConvergence.value(); - mc.setAutoConvergence(kvmAutoConvergence); - mc.setHostGuid(dest.getHost().getGuid()); + final MigrateCommand mc = buildMigrateCommand(vm, to, dest, pfma, null); try { final Answer ma = _agentMgr.send(vm.getLastHostId(), mc); diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index 1419ae36d25..a63aa52799d 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java @@ -31,6 +31,7 @@ import java.util.concurrent.TimeUnit; import javax.inject.Inject; +import com.cloud.agent.api.PrepareForMigrationAnswer; import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; @@ -1884,9 +1885,10 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { } PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(vmTO); + Answer pfma; try { - Answer pfma = agentManager.send(destHost.getId(), pfmc); + pfma = agentManager.send(destHost.getId(), pfmc); if (pfma == null || !pfma.getResult()) { String details = pfma != null ? pfma.getDetails() : "null answer returned"; @@ -1894,8 +1896,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { throw new AgentUnavailableException(msg, destHost.getId()); } - } - catch (final OperationTimedoutException e) { + } catch (final OperationTimedoutException e) { throw new AgentUnavailableException("Operation timed out", destHost.getId()); } @@ -1911,6 +1912,12 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { migrateCommand.setMigrateStorageManaged(managedStorageDestination); migrateCommand.setMigrateNonSharedInc(migrateNonSharedInc); + Integer newVmCpuShares = ((PrepareForMigrationAnswer) pfma).getNewVmCpuShares(); + if (newVmCpuShares != null) { + LOGGER.debug(String.format("Setting CPU shares to [%d] as part of migrate VM with volumes command for VM [%s].", newVmCpuShares, vmTO)); + migrateCommand.setNewVmCpuShares(newVmCpuShares); + } + boolean kvmAutoConvergence = StorageManager.KvmAutoConvergence.value(); migrateCommand.setAutoConvergence(kvmAutoConvergence); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index b7611cd07bb..a3bee2f4134 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -72,6 +72,7 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.BooleanUtils; import org.apache.commons.lang.math.NumberUtils; +import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.apache.xerces.impl.xpath.regex.Match; @@ -472,6 +473,14 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv */ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live"; + private static int hostCpuMaxCapacity = 0; + + private static final int CGROUP_V2_UPPER_LIMIT = 10000; + + private static final String COMMAND_GET_CGROUP_HOST_VERSION = "stat -fc %T /sys/fs/cgroup/"; + + public static final String CGROUP_V2 = "cgroup2fs"; + protected long getHypervisorLibvirtVersion() { return _hypervisorLibvirtVersion; } @@ -547,6 +556,18 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return new ExecutionResult(true, null); } + /** + * @return the host CPU max capacity according to the method {@link LibvirtComputingResource#calculateHostCpuMaxCapacity(int, Long)}; if the host utilizes cgroup v1, this + * value is 0. + */ + public int getHostCpuMaxCapacity() { + return hostCpuMaxCapacity; + } + + public void setHostCpuMaxCapacity(int hostCpuMaxCapacity) { + LibvirtComputingResource.hostCpuMaxCapacity = hostCpuMaxCapacity; + } + public LibvirtKvmAgentHook getTransformer() throws IOException { return new LibvirtKvmAgentHook(_agentHooksBasedir, _agentHooksLibvirtXmlScript, _agentHooksLibvirtXmlMethod); } @@ -2673,12 +2694,41 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv */ protected CpuTuneDef createCpuTuneDef(VirtualMachineTO vmTO) { CpuTuneDef ctd = new CpuTuneDef(); - int shares = vmTO.getCpus() * (vmTO.getMinSpeed() != null ? vmTO.getMinSpeed() : vmTO.getSpeed()); - ctd.setShares(shares); + ctd.setShares(calculateCpuShares(vmTO)); setQuotaAndPeriod(vmTO, ctd); return ctd; } + /** + * Calculates the VM CPU shares considering the cgroup version of the host. + *
    + *
  • + * If the host utilize cgroup v1, then, the CPU shares is calculated as VM CPU shares = CPU cores * CPU frequency. + *
    • + *
  • + * If the host utilize cgroup v2, the CPU shares calculation considers the cgroup v2 upper limit of 10,000, and a linear scale conversion is applied + * considering the maximum host CPU shares (i.e. using the number of CPU cores and CPU nominal frequency of the host). Therefore, the VM CPU shares is calculated as + * VM CPU shares = (VM requested shares * cgroup upper limit) / host max shares. + *
    • + *
+ */ + public int calculateCpuShares(VirtualMachineTO vmTO) { + int vCpus = vmTO.getCpus(); + int cpuSpeed = ObjectUtils.defaultIfNull(vmTO.getMinSpeed(), vmTO.getSpeed()); + int requestedCpuShares = vCpus * cpuSpeed; + int hostCpuMaxCapacity = getHostCpuMaxCapacity(); + + if (hostCpuMaxCapacity > 0) { + int updatedCpuShares = (int) Math.ceil((requestedCpuShares * CGROUP_V2_UPPER_LIMIT) / (double) hostCpuMaxCapacity); + s_logger.debug(String.format("This host utilizes cgroupv2 (as the max shares value is [%s]), thus, the VM requested shares of [%s] will be converted to " + + "consider the host limits; the new CPU shares value is [%s].", hostCpuMaxCapacity, requestedCpuShares, updatedCpuShares)); + return updatedCpuShares; + } + s_logger.debug(String.format("This host does not have a maximum CPU shares set; therefore, this host utilizes cgroupv1 and the VM requested CPU shares [%s] will not be " + + "converted.", requestedCpuShares)); + return requestedCpuShares; + } + private CpuModeDef createCpuModeDef(VirtualMachineTO vmTO, int vcpus) { final CpuModeDef cmd = new CpuModeDef(); cmd.setMode(_guestCpuMode); @@ -3469,8 +3519,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv @Override public StartupCommand[] initialize() { - final KVMHostInfo info = new KVMHostInfo(_dom0MinMem, _dom0OvercommitMem, _manualCpuSpeed); + calculateHostCpuMaxCapacity(info.getCpus(), info.getCpuSpeed()); String capabilities = String.join(",", info.getCapabilities()); if (dpdkSupport) { @@ -3514,6 +3564,32 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return startupCommandsArray; } + /** + * Calculates and sets the host CPU max capacity according to the cgroup version of the host. + *
    + *
  • + * cgroup v1: the max CPU capacity for the host is set to 0. + *
    • + *
  • + * cgroup v2: the max CPU capacity for the host is the value of cpuCores * cpuSpeed. + *
    • + *
+ */ + protected void calculateHostCpuMaxCapacity(int cpuCores, Long cpuSpeed) { + String output = Script.runSimpleBashScript(COMMAND_GET_CGROUP_HOST_VERSION); + s_logger.info(String.format("Host uses control group [%s].", output)); + + if (!CGROUP_V2.equals(output)) { + s_logger.info(String.format("Setting host CPU max capacity to 0, as it uses cgroup v1.", getHostCpuMaxCapacity())); + setHostCpuMaxCapacity(0); + return; + } + + s_logger.info(String.format("Calculating the max shares of the host.")); + setHostCpuMaxCapacity(cpuCores * cpuSpeed.intValue()); + s_logger.info(String.format("The max shares of the host is [%d].", getHostCpuMaxCapacity())); + } + private StartupStorageCommand createLocalStoragePool(String localStoragePath, String localStorageUUID, StartupRoutingCommand cmd) { StartupStorageCommand sscmd = null; try { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index d0ab77829af..fb526626ef8 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -23,6 +23,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; import java.util.Set; @@ -211,6 +212,8 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper + *
  • + * If both hosts utilize cgroup v1; then, the shares value of the VM is equal in both hosts, and there is no need to update the VM CPU shares value for the + * migration.
    • + *
  • + * If, at least, one of the hosts utilize cgroup v2, the VM CPU shares must be recalculated for the migration, accordingly to + * method {@link LibvirtComputingResource#calculateCpuShares(VirtualMachineTO)}. + *
    • + * + */ + protected String updateVmSharesIfNeeded(MigrateCommand migrateCommand, String xmlDesc, LibvirtComputingResource libvirtComputingResource) + throws ParserConfigurationException, IOException, SAXException, TransformerException { + Integer newVmCpuShares = migrateCommand.getNewVmCpuShares(); + int currentCpuShares = libvirtComputingResource.calculateCpuShares(migrateCommand.getVirtualMachine()); + + if (newVmCpuShares == currentCpuShares) { + s_logger.info(String.format("Current CPU shares [%s] is equal in both hosts; therefore, there is no need to update the CPU shares for the new host.", + currentCpuShares)); + return xmlDesc; + } + + InputStream inputStream = IOUtils.toInputStream(xmlDesc, StandardCharsets.UTF_8); + DocumentBuilderFactory docFactory = ParserUtils.getSaferDocumentBuilderFactory(); + DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); + Document document = docBuilder.parse(inputStream); + + Element root = document.getDocumentElement(); + Node sharesNode = root.getElementsByTagName("shares").item(0); + String currentShares = sharesNode.getTextContent(); + + s_logger.info(String.format("VM [%s] will have CPU shares altered from [%s] to [%s] as part of migration because the cgroups version differs between hosts.", + migrateCommand.getVmName(), currentShares, newVmCpuShares)); + sharesNode.setTextContent(String.valueOf(newVmCpuShares)); + return getXml(document); + } + /** * Replace DPDK source path and target before migrations */ diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java index 9109d579c5b..3f281e54bba 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java @@ -122,11 +122,7 @@ public final class LibvirtPrepareForMigrationCommandWrapper extends CommandWrapp return new PrepareForMigrationAnswer(command, "failed to connect physical disks to host"); } - PrepareForMigrationAnswer answer = new PrepareForMigrationAnswer(command); - if (MapUtils.isNotEmpty(dpdkInterfaceMapping)) { - answer.setDpdkInterfaceMapping(dpdkInterfaceMapping); - } - return answer; + return createPrepareForMigrationAnswer(command, dpdkInterfaceMapping, libvirtComputingResource, vm); } catch (final LibvirtException | CloudRuntimeException | InternalErrorException | URISyntaxException e) { if (MapUtils.isNotEmpty(dpdkInterfaceMapping)) { for (DpdkTO to : dpdkInterfaceMapping.values()) { @@ -143,6 +139,22 @@ public final class LibvirtPrepareForMigrationCommandWrapper extends CommandWrapp } } + protected PrepareForMigrationAnswer createPrepareForMigrationAnswer(PrepareForMigrationCommand command, Map dpdkInterfaceMapping, + LibvirtComputingResource libvirtComputingResource, VirtualMachineTO vm) { + PrepareForMigrationAnswer answer = new PrepareForMigrationAnswer(command); + + if (MapUtils.isNotEmpty(dpdkInterfaceMapping)) { + s_logger.debug(String.format("Setting DPDK interface for the migration of VM [%s].", vm)); + answer.setDpdkInterfaceMapping(dpdkInterfaceMapping); + } + + int newCpuShares = libvirtComputingResource.calculateCpuShares(vm); + s_logger.debug(String.format("Setting CPU shares to [%s] for the migration of VM [%s].", newCpuShares, vm)); + answer.setNewVmCpuShares(newCpuShares); + + return answer; + } + private Answer handleRollback(PrepareForMigrationCommand command, LibvirtComputingResource libvirtComputingResource) { KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr(); VirtualMachineTO vmTO = command.getVirtualMachine(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java index 963d13bff24..79d43ba2735 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapper.java @@ -39,8 +39,7 @@ public class LibvirtScaleVmCommandWrapper extends CommandWrapper\n" + ""; + @Mock + MigrateCommand migrateCommandMock; + + @Mock + LibvirtComputingResource libvirtComputingResourceMock; + + @Mock + VirtualMachineTO virtualMachineTOMock; + + @Spy LibvirtMigrateCommandWrapper libvirtMigrateCmdWrapper = new LibvirtMigrateCommandWrapper(); final String memInfo = "MemTotal: 5830236 kB\n" + @@ -871,4 +888,67 @@ public class LibvirtMigrateCommandWrapperTest { Assert.assertTrue(replaced.contains("csdpdk-7")); Assert.assertFalse(replaced.contains("csdpdk-1")); } + + @Test + public void updateVmSharesIfNeededTestNewCpuSharesEqualCurrentSharesShouldNotUpdateVmShares() throws ParserConfigurationException, IOException, TransformerException, + SAXException { + int newVmCpuShares = 1000; + int currentVmCpuShares = 1000; + + Mockito.doReturn(newVmCpuShares).when(migrateCommandMock).getNewVmCpuShares(); + Mockito.doReturn(virtualMachineTOMock).when(migrateCommandMock).getVirtualMachine(); + Mockito.doReturn(currentVmCpuShares).when(libvirtComputingResourceMock).calculateCpuShares(virtualMachineTOMock); + + String finalXml = libvirtMigrateCmdWrapper.updateVmSharesIfNeeded(migrateCommandMock, fullfile, libvirtComputingResourceMock); + + Assert.assertEquals(finalXml, fullfile); + } + + @Test + public void updateVmSharesIfNeededTestNewCpuSharesHigherThanCurrentSharesShouldUpdateVmShares() throws ParserConfigurationException, IOException, TransformerException, + SAXException { + int newVmCpuShares = 2000; + int currentVmCpuShares = 1000; + + Mockito.doReturn(newVmCpuShares).when(migrateCommandMock).getNewVmCpuShares(); + Mockito.doReturn(virtualMachineTOMock).when(migrateCommandMock).getVirtualMachine(); + Mockito.doReturn(currentVmCpuShares).when(libvirtComputingResourceMock).calculateCpuShares(virtualMachineTOMock); + + String finalXml = libvirtMigrateCmdWrapper.updateVmSharesIfNeeded(migrateCommandMock, fullfile, libvirtComputingResourceMock); + + InputStream inputStream = IOUtils.toInputStream(finalXml, StandardCharsets.UTF_8); + DocumentBuilderFactory docFactory = ParserUtils.getSaferDocumentBuilderFactory(); + DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); + Document document = docBuilder.parse(inputStream); + + Element root = document.getDocumentElement(); + Node sharesNode = root.getElementsByTagName("shares").item(0); + int updateShares = Integer.parseInt(sharesNode.getTextContent()); + + Assert.assertEquals(updateShares, newVmCpuShares); + } + + @Test + public void updateVmSharesIfNeededTestNewCpuSharesLowerThanCurrentSharesShouldUpdateVmShares() throws ParserConfigurationException, IOException, TransformerException, + SAXException { + int newVmCpuShares = 500; + int currentVmCpuShares = 1000; + + Mockito.doReturn(newVmCpuShares).when(migrateCommandMock).getNewVmCpuShares(); + Mockito.doReturn(virtualMachineTOMock).when(migrateCommandMock).getVirtualMachine(); + Mockito.doReturn(currentVmCpuShares).when(libvirtComputingResourceMock).calculateCpuShares(virtualMachineTOMock); + + String finalXml = libvirtMigrateCmdWrapper.updateVmSharesIfNeeded(migrateCommandMock, fullfile, libvirtComputingResourceMock); + + InputStream inputStream = IOUtils.toInputStream(finalXml, StandardCharsets.UTF_8); + DocumentBuilderFactory docFactory = ParserUtils.getSaferDocumentBuilderFactory(); + DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); + Document document = docBuilder.parse(inputStream); + + Element root = document.getDocumentElement(); + Node sharesNode = root.getElementsByTagName("shares").item(0); + int updateShares = Integer.parseInt(sharesNode.getTextContent()); + + Assert.assertEquals(updateShares, newVmCpuShares); + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapperTest.java new file mode 100644 index 00000000000..5530819c2e4 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapperTest.java @@ -0,0 +1,75 @@ +// +// 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 com.cloud.hypervisor.kvm.resource.wrapper; + +import com.cloud.agent.api.PrepareForMigrationAnswer; +import com.cloud.agent.api.PrepareForMigrationCommand; +import com.cloud.agent.api.to.DpdkTO; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import java.util.HashMap; +import java.util.Map; + +@RunWith(PowerMockRunner.class) +@PrepareForTest(value = {LibvirtPrepareForMigrationCommandWrapper.class}) +public class LibvirtPrepareForMigrationCommandWrapperTest { + + @Mock + LibvirtComputingResource libvirtComputingResourceMock; + + @Mock + PrepareForMigrationCommand prepareForMigrationCommandMock; + + @Mock + VirtualMachineTO virtualMachineTOMock; + + @Spy + LibvirtPrepareForMigrationCommandWrapper libvirtPrepareForMigrationCommandWrapperSpy = new LibvirtPrepareForMigrationCommandWrapper(); + + @Test + public void createPrepareForMigrationAnswerTestDpdkInterfaceNotEmptyShouldSetParamOnAnswer() { + Map dpdkInterfaceMapping = new HashMap<>(); + dpdkInterfaceMapping.put("Interface", new DpdkTO()); + + PrepareForMigrationAnswer prepareForMigrationAnswer = libvirtPrepareForMigrationCommandWrapperSpy.createPrepareForMigrationAnswer(prepareForMigrationCommandMock, dpdkInterfaceMapping, libvirtComputingResourceMock, + virtualMachineTOMock); + + Assert.assertEquals(prepareForMigrationAnswer.getDpdkInterfaceMapping(), dpdkInterfaceMapping); + } + + @Test + public void createPrepareForMigrationAnswerTestVerifyThatCpuSharesIsSet() { + int cpuShares = 1000; + Mockito.doReturn(cpuShares).when(libvirtComputingResourceMock).calculateCpuShares(virtualMachineTOMock); + PrepareForMigrationAnswer prepareForMigrationAnswer = libvirtPrepareForMigrationCommandWrapperSpy.createPrepareForMigrationAnswer(prepareForMigrationCommandMock,null, + libvirtComputingResourceMock, virtualMachineTOMock); + + Assert.assertEquals(cpuShares, prepareForMigrationAnswer.getNewVmCpuShares().intValue()); + } +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapperTest.java index fb963e87ed4..56f99d41abd 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtScaleVmCommandWrapperTest.java @@ -207,9 +207,11 @@ public class LibvirtScaleVmCommandWrapperTest extends TestCase { @Test public void validateExecuteHandleLibvirtException() throws LibvirtException { String errorMessage = ""; + int shares = vmTo.getCpus() * vmTo.getSpeed(); Mockito.doReturn(vmTo).when(scaleVmCommandMock).getVirtualMachine(); Mockito.doReturn(libvirtUtilitiesHelperMock).when(libvirtComputingResourceMock).getLibvirtUtilitiesHelper(); + Mockito.doReturn(shares).when(libvirtComputingResourceMock).calculateCpuShares(vmTo); Mockito.doThrow(libvirtException).when(libvirtUtilitiesHelperMock).getConnectionByVmName(Mockito.anyString()); Mockito.doReturn(errorMessage).when(libvirtException).getMessage(); @@ -222,9 +224,12 @@ public class LibvirtScaleVmCommandWrapperTest extends TestCase { @Test public void validateExecuteSuccessfully() throws LibvirtException { + int shares = vmTo.getCpus() * vmTo.getSpeed(); + Mockito.doReturn(vmTo).when(scaleVmCommandMock).getVirtualMachine(); Mockito.doReturn(libvirtUtilitiesHelperMock).when(libvirtComputingResourceMock).getLibvirtUtilitiesHelper(); Mockito.doReturn(connectMock).when(libvirtUtilitiesHelperMock).getConnectionByVmName(Mockito.anyString()); + Mockito.doReturn(shares).when(libvirtComputingResourceMock).calculateCpuShares(vmTo); Mockito.doReturn(domainMock).when(connectMock).domainLookupByName(Mockito.anyString()); Mockito.doNothing().when(libvirtScaleVmCommandWrapperSpy).scaleMemory(Mockito.any(), Mockito.anyLong(), Mockito.anyString()); Mockito.doNothing().when(libvirtScaleVmCommandWrapperSpy).scaleVcpus(Mockito.any(), Mockito.anyInt(), Mockito.anyString());