From dc975dff958520234624bf025df5ac7a5072c88d Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Mon, 23 May 2022 08:11:14 -0300 Subject: [PATCH 1/2] [KVM] Enable IOURING only when it is available on the host (#6399) * [KVM] Disable IOURING by default on agents * Refactor * Remove agent property for iouring * Restore property * Refactor suse check and enable on ubuntu by default * Refactor irrespective of guest OS * Improvement * Logs and new path * Refactor condition to enable iouring * Improve condition * Refactor property check * Improvement * Doc comment * Extend comment * Move method * Add log --- agent/conf/agent.properties | 2 +- .../agent/properties/AgentProperties.java | 7 -- .../resource/LibvirtComputingResource.java | 74 ++++++++++++++++++- 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 519b875f912..e4468a14616 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -286,5 +286,5 @@ iscsi.session.cleanup.enabled=false # Enable manually setting CPU's topology on KVM's VM. # enable.manually.setting.cpu.topology.on.kvm.vm=true -# Enable/disable IO driver for Qemu / It's enabled by default on KVM agents +# Enable/disable IO driver for Qemu (in case it is not set CloudStack can also detect if its supported by qemu) # enable.io.uring=true diff --git a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java index 980c9b080aa..657876c13ac 100644 --- a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java +++ b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java @@ -54,13 +54,6 @@ public class AgentProperties{ */ public static final Property ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM = new Property("enable.manually.setting.cpu.topology.on.kvm.vm", true); - /** - * Enable manually IO driver on KVM's VM.
- * Data type: boolean.
- * Default value: true. - */ - public static final Property ENABLE_IO_URING = new Property("enable.io.uring", true); - public static class Property { private final String name; private final T defaultValue; 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 b78749bb43b..72e3e0027f2 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 @@ -313,6 +313,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv public final static String HOST_CACHE_PATH_PARAMETER = "host.cache.location"; public final static String CONFIG_DIR = "config"; + private boolean enableIoUring; + private final static String ENABLE_IO_URING_PROPERTY = "enable.io.uring"; public static final String BASH_SCRIPT_PATH = "/bin/bash"; @@ -1132,6 +1134,12 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv s_logger.trace("Ignoring libvirt error.", e); } + // Enable/disable IO driver for Qemu (in case it is not set CloudStack can also detect if its supported by qemu) + // Do not remove - switching it to AgentProperties.Property may require accepting null values for the properties default value + String enableIoUringConfig = (String) params.get(ENABLE_IO_URING_PROPERTY); + enableIoUring = isIoUringEnabled(enableIoUringConfig); + s_logger.info("IO uring driver for Qemu: " + (enableIoUring ? "enabled" : "disabled")); + final String cpuArchOverride = (String)params.get("guest.cpu.arch"); if (!Strings.isNullOrEmpty(cpuArchOverride)) { _guestCpuArch = cpuArchOverride; @@ -2949,6 +2957,33 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return storagePool.getPhysicalDisk(data.getPath()); } + /** + * Check if IO_URING is supported by qemu + */ + protected boolean isIoUringSupportedByQemu() { + s_logger.debug("Checking if iouring is supported"); + String command = getIoUringCheckCommand(); + if (org.apache.commons.lang3.StringUtils.isBlank(command)) { + s_logger.debug("Could not check iouring support, disabling it"); + return false; + } + int exitValue = executeBashScriptAndRetrieveExitValue(command); + return exitValue == 0; + } + + protected String getIoUringCheckCommand() { + String[] qemuPaths = { "/usr/bin/qemu-system-x86_64", "/usr/libexec/qemu-kvm", "/usr/bin/qemu-kvm" }; + for (String qemuPath : qemuPaths) { + File file = new File(qemuPath); + if (file.exists()) { + String cmd = String.format("ldd %s | grep -Eqe '[[:space:]]liburing\\.so'", qemuPath); + s_logger.debug("Using the check command: " + cmd); + return cmd; + } + } + return null; + } + /** * Set Disk IO Driver, if supported by the Libvirt/Qemu version. * IO Driver works for: @@ -2956,13 +2991,34 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv * (ii) Libvirt >= 6.3.0 */ protected void setDiskIoDriver(DiskDef disk) { - if (getHypervisorLibvirtVersion() >= HYPERVISOR_LIBVIRT_VERSION_SUPPORTS_IO_URING - && getHypervisorQemuVersion() >= HYPERVISOR_QEMU_VERSION_SUPPORTS_IO_URING - && AgentPropertiesFileHandler.getPropertyValue(AgentProperties.ENABLE_IO_URING)) { + if (enableIoUring) { disk.setIoDriver(DiskDef.IoDriver.IOURING); } } + /** + * IO_URING supported if the property 'enable.io.uring' is set to true OR it is supported by qemu + */ + private boolean isIoUringEnabled(String enableIoUringConfig) { + boolean meetRequirements = getHypervisorLibvirtVersion() >= HYPERVISOR_LIBVIRT_VERSION_SUPPORTS_IO_URING + && getHypervisorQemuVersion() >= HYPERVISOR_QEMU_VERSION_SUPPORTS_IO_URING; + if (!meetRequirements) { + return false; + } + return enableIoUringConfig != null ? + Boolean.parseBoolean(enableIoUringConfig): + (isBaseOsUbuntu() || isIoUringSupportedByQemu()); + } + + private boolean isBaseOsUbuntu() { + Map versionString = getVersionStrings(); + String hostKey = "Host.OS"; + if (MapUtils.isEmpty(versionString) || !versionString.containsKey(hostKey) || versionString.get(hostKey) == null) { + return false; + } + return versionString.get(hostKey).equalsIgnoreCase("ubuntu"); + } + private KVMPhysicalDisk getPhysicalDiskFromNfsStore(String dataStoreUrl, DataTO data) { final String volPath = dataStoreUrl + File.separator + data.getPath(); final int index = volPath.lastIndexOf("/"); @@ -3826,10 +3882,20 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } private String executeBashScript(final String script) { + return createScript(script).execute(); + } + + private Script createScript(final String script) { final Script command = new Script("/bin/bash", _timeout, s_logger); command.add("-c"); command.add(script); - return command.execute(); + return command; + } + + private int executeBashScriptAndRetrieveExitValue(final String script) { + Script command = createScript(script); + command.execute(); + return command.getExitValue(); } public List getVmNetworkStat(Connect conn, String vmName) throws LibvirtException { From b1c8b5ab370f681fa0a92b3f185cab95bd9daad5 Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Mon, 23 May 2022 08:12:49 -0300 Subject: [PATCH 2/2] [KVM] Fix VM migration error due to VNC password on libvirt limiting versions (#6404) * [KVM] Fix VM migration error due to VNC password on libvirt limiting versions * Fix passwd value * Simplify implementation --- .../wrapper/LibvirtMigrateCommandWrapper.java | 12 +++++++++-- .../LibvirtMigrateCommandWrapperTest.java | 21 ++++++++++--------- 2 files changed, 21 insertions(+), 12 deletions(-) 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 ab448df3009..ed3b0481819 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 @@ -149,7 +149,11 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper * @param xmlDesc the qemu xml description * @param target the ip address to migrate to + * @param vncPassword if set, the VNC password truncated to 8 characters * @return the new xmlDesc */ - String replaceIpForVNCInDescFile(String xmlDesc, final String target) { + String replaceIpForVNCInDescFileAndNormalizePassword(String xmlDesc, final String target, String vncPassword) { final int begin = xmlDesc.indexOf(GRAPHICS_ELEM_START); if (begin >= 0) { final int end = xmlDesc.lastIndexOf(GRAPHICS_ELEM_END) + GRAPHICS_ELEM_END.length(); @@ -460,6 +465,9 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper" + - " " + + " " + " " + " " + " " + @@ -588,22 +588,23 @@ public class LibvirtMigrateCommandWrapperTest { final String expectedXmlDesc = "" + " " + - " " + + " " + " " + " " + " " + ""; final String targetIp = "10.10.10.10"; - final String result = libvirtMigrateCmdWrapper.replaceIpForVNCInDescFile(xmlDesc, targetIp); + final String password = "12345678"; + final String result = libvirtMigrateCmdWrapper.replaceIpForVNCInDescFileAndNormalizePassword(xmlDesc, targetIp, password); assertTrue("transformation does not live up to expectation:\n" + result, expectedXmlDesc.equals(result)); } @Test - public void testReplaceFqdnForVNCInDesc() { + public void testReplaceFqdnAndPasswordForVNCInDesc() { final String xmlDesc = "" + " " + - " " + + " " + " " + " " + " " + @@ -611,13 +612,14 @@ public class LibvirtMigrateCommandWrapperTest { final String expectedXmlDesc = "" + " " + - " " + + " " + " " + " " + " " + ""; final String targetIp = "localhost.localdomain"; - final String result = libvirtMigrateCmdWrapper.replaceIpForVNCInDescFile(xmlDesc, targetIp); + final String password = "12345678"; + final String result = libvirtMigrateCmdWrapper.replaceIpForVNCInDescFileAndNormalizePassword(xmlDesc, targetIp, password); assertTrue("transformation does not live up to expectation:\n" + result, expectedXmlDesc.equals(result)); } @@ -789,5 +791,4 @@ public class LibvirtMigrateCommandWrapperTest { Assert.assertTrue(replaced.contains("csdpdk-7")); Assert.assertFalse(replaced.contains("csdpdk-1")); } - }