Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.4k
[KVM] Enable IOURING only when it is available on the host#6399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
f9aa91cfad8a571d6d1b0c11cd1b261e2750e76ca69e05746b349e75a74a4a809c1f44a3ab1a388b36660c8a6aa3835e8bfa694c2c16e752File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 boolean configure(final String name, final Map<String, Object> params) th | ||
| 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")); | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. one small suggestion if you want to log if the IO uring is enabled/disabled Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes please, let's do the proper logging. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, thanks | ||
| final String cpuArchOverride = (String)params.get("guest.cpu.arch"); | ||
| if (!Strings.isNullOrEmpty(cpuArchOverride)) { | ||
| _guestCpuArch = cpuArchOverride; | ||
| @@ -2949,20 +2957,68 @@ private KVMPhysicalDisk getPhysicalDiskPrimaryStore(PrimaryDataStoreTO primaryDa | ||
| 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); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this reliable @nvazquez cc others? Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rohityadavcloud LGTM | ||
| 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: | ||
| * (i) Qemu >= 5.0; | ||
| * (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<String, String> 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 List<DiskDef> getDisks(final Connect conn, final String vmName) { | ||
| } | ||
| 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<VmNetworkStatsEntry> getVmNetworkStat(Connect conn, String vmName) throws LibvirtException { | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good to leave this option also ... in case the check isn't enough (in future) or the user wants to disable the
io_uringeven if it is supportedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nvazquez +1 for optional properties entry, people might want to disable it, it's new tech, might misbehave etc.
agent.properties should have the last word on whether it's enabled or not, imho, if it's not too much bother.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done thanks @slavkap@NuxRo