-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[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
Changes from all commits
f9aa91c
fad8a57
1d6d1b0
c11cd1b
261e275
0e76ca6
9e05746
b349e75
a74a4a8
09c1f44
a3ab1a3
88b3666
0c8a6aa
3835e8b
fa694c2
c16e752
File filter
Filter by extension
Conversations
Jump to
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.
Contributor
Author
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