[KVM] Enable IOURING only when it is available on the host #6399

nvazquez · 2022-05-19T11:27:31Z

Description

This PR enables IO_URING only when it is available on the hosts, as being discussed on #6253 (comment)

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

weizhouapache

code lgtm

nvazquez · 2022-05-19T11:34:09Z

@blueorangutan package

blueorangutan · 2022-05-19T11:35:03Z

@nvazquez a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

GabrielBrascher · 2022-05-19T11:51:37Z

@nvazquez maybe we should disable IO_URING to Rhel environments only, as these are the affected ones.
Ubuntu environments normally have the correct features mapped by Libvirt/Qemu.

Unfortunately, as we have been seeing in multiple KVM implementations, RedHat has a different route and tends to not keep up the pace with most of the new features that Qemu/Libvirt present. In this regard, there is no perfect world and we will always be compromising one side or the other (as many other features that had to be blocked due to Rhel).

I see that global settings are indeed a good way of disabling such features, although we can easily get lost with the growth of multiple features that need to be enabled/disabled accordingly to CentOS vs Ubuntu.

We might need better options to proceed with this.

With that said, your approach looks fine with me, as a quick fix.
A note in the upgrade process, raising how to proceed to enable in case the KVM is running in Ubuntu (or how to proceed to disable in the case of Rhel) could be interesting.

GabrielBrascher

Thanks for the PR @nvazquez! LGTM

A documentation point raising this might be good, in order to avoid users with Ubuntu and the specific Libvirt/Qemu setup missing this feature.

Although I do not believe this is going to be a good way for handling the multiple differences between CentOS & Ubuntu in the medium/long term ... it is what it is :)

For another time & future implementations: we might need to revisit some parts where we check for the Libvirt version as well and run different checks.

rohityadavcloud · 2022-05-19T12:05:37Z

@nvazquez cc @GabrielBrascher @wido what about Suse? If we can reliably detect the distro then the default behaviour (which should be documented somewhere) can be: if is Ubuntu, then enable io_uring, otherwise, disable it by default for other distros, and use the agent.properties always at the override.

blueorangutan · 2022-05-19T12:09:06Z

Packaging result: ✔️ el7 ✔️ el8 ✖️ debian ✔️ suse15. SL-JID 3445

wido · 2022-05-19T12:15:38Z

io_uring brings a massive performance improvement to end-users. It's RHEL and thus CentOS/Alma/Rocky which have this problem. Ubuntu does not have this issue.

I do understand the problem here where CloudStack doesn't work out of the box, but if CloudStack becomes a project where you have a super long list of tunables/settings to get the optimal performance out of the system you will see users having endless tuning to do.

In the future io_uring should really be enabled by default or at least we should enable it if supported by the Qemu version. Otherwise users won't get the performance they want/expect.

Proxmox nowadays enables io_uring by default for all VMs being created. It's just such a massive performance boost.

GabrielBrascher · 2022-05-19T12:16:41Z

@rohityadavcloud your idea would be great indeed in order to present the best features to each distro.

It is a shame that not all OS distros support some great features that Libvirt/Qemu present.
We've been always retrieving the Libvirt / Qemu versions in the codebase to decide which way to proceed when adding KVM features. I tend to follow Libvirt versions and release notes. Unfortunately, this has not been the best way in this particular case.

For us, as developers, it will always be a struggle to map all the different environments when developing a feature.

andrijapanicsb · 2022-05-19T12:22:55Z

@GabrielBrascher @wido - we do understand that there are huge performance benefits, but that is for a small subset of OS-es that support it.

If you can propose a better solution, than this one here (to disable by default) where existing EL8 implementations are not broken (and that is the vast majority or ACS users today) - I'm for it (i.e. if we can reliably detect where to enable and where not) - otherwise we can't just go around and break things for 95% of the users...

GabrielBrascher · 2022-05-19T12:42:51Z

@andrijapanicsb I totally understand it. I am sorry in the case I was not clear.
I am +1 on proceeding with the current route and merging this, I do understand that this impact a relevant amount of our users.

But I wanted to take the opportunity to raise the concerns.
We've been seeing this happening other times, and it will stay happening many other times in the future.

Supporting a wide set of technologies has been a great strength of CloudStack, I do not want to take this out.
But we might need to improve the way we design some features in order to explore the best of each environment that CloudStack will be running into.

Otherwise, inevitably, people will start blaming CloudStack / KVM for "poor performance" and walk away to other options that suit best them. While they could actually be having the best for their environment.

Right now, I don't have any option on the table to fix it. But it is something good to keep in mind (for another time).

rohityadavcloud · 2022-05-19T12:46:44Z

@nvazquez based on this, possibly the best compromise is to have io_uring enabled by default for Ubuntu based KVM hosts, but disabled otherwise. Can you amend the PR with this logic and propose a doc PR to document the default behaviour by distro? Thanks.

slavkap · 2022-05-19T12:53:08Z

Hi all, we could try by parsing the stderr (but I know it's ugly) and enable/disable according to the output:

Ubuntu

sudo /usr/bin/qemu-system-x86_64 -drive aio=io_uring
qemu-system-x86_64: Device needs media, but drive is empty

Rocky

 /usr/libexec/qemu-kvm -drive aio=io_uring 
qemu-kvm: -drive aio=io_uring: invalid aio option

nvazquez · 2022-05-19T12:54:18Z

Have discussed with @slavkap and @GabrielBrascher about a possible fix to detect if iouring is enabled by a call to qemu-kvm with the aio parameter. Exploring this option and will be updating the PR

blueorangutan · 2022-05-20T16:47:27Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3460

nvazquez · 2022-05-20T16:53:04Z

@blueorangutan test

blueorangutan · 2022-05-20T16:54:02Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2022-05-21T01:44:29Z

Trillian test result (tid-4222)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30149 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6399-t4222-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

slavkap · 2022-05-21T07:39:31Z

...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

+        String enableIoUringConfig = (String) params.get(ENABLE_IO_URING_PROPERTY);
+        enableIoUring = isIoUringEnabled(enableIoUringConfig);


Suggested change

String enableIoUringConfig = (String) params.get(ENABLE_IO_URING_PROPERTY);

enableIoUring = isIoUringEnabled(enableIoUringConfig);

enableIoUring = isIoUringEnabled(enableIoUringConfig);

needs to be moved after this part of the code

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

Lines 1126 to 1133 in 363a2cf

try {

_hvVersion = conn.getVersion();

_hvVersion = _hvVersion % 1000000 / 1000;

_hypervisorLibvirtVersion = conn.getLibVirVersion();

_hypervisorQemuVersion = conn.getVersion();

} catch (final LibvirtException e) {

s_logger.trace("Ignoring libvirt error.", e);

}

otherwise it will be always false

Thanks @slavkap, done

nvazquez · 2022-05-21T11:45:08Z

@blueorangutan package

blueorangutan · 2022-05-21T11:46:03Z

@nvazquez a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2022-05-21T12:22:58Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3463

nvazquez · 2022-05-21T13:26:53Z

@blueorangutan test

blueorangutan · 2022-05-21T13:28:03Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2022-05-21T13:32:30Z

Trillian Build Failed (tid-4224)

rohityadavcloud · 2022-05-21T17:10:54Z

@blueorangutan test

blueorangutan · 2022-05-21T17:12:03Z

@rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2022-05-21T17:16:37Z

Trillian Build Failed (tid-4225)

rohityadavcloud · 2022-05-22T02:14:41Z

@blueorangutan test

blueorangutan · 2022-05-22T02:15:03Z

@rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2022-05-22T02:20:22Z

Trillian Build Failed (tid-4226)

slavkap · 2022-05-23T05:30:38Z

...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

+        // 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);
+


one small suggestion if you want to log if the IO uring is enabled/disabled

Yes please, let's do the proper logging.

Done, thanks

slavkap

code LGTM! Rebased on main and tested on Rocky - libvirt/qemu versions

Compiled against library: libvirt 8.0.0
Using library: libvirt 8.0.0
Using API: QEMU 8.0.0
Running hypervisor: QEMU 6.2.0

and with set/not set enable.io.uring in the agent properties

andrijapanicsb · 2022-05-23T07:55:43Z

@blueorangutan test

blueorangutan · 2022-05-23T07:57:03Z

@andrijapanicsb a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2022-05-23T18:56:39Z

Trillian test result (tid-4231)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38318 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6399-t4231-kvm-centos7.zip
Smoke tests completed. 91 look OK, 1 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_disable_oobm_ha_state_ineligible	`Error`	1512.29	test_hostha_kvm.py

* [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

* Extract the IO_URING configuration into the agent.properties (apache#6253) When using advanced virtualization the IO Driver is not supported. The admin will decide if want to enable/disable this configuration from agent.properties file. The default value is true * kvm: truncate vnc password to 8 chars (apache#6244) This PR truncates the vnc password of kvm vms to 8 chars to support latest versions of libvirt. * merge fix Signed-off-by: Abhishek Kumar <[email protected]> * [KVM] Enable IOURING only when it is available on the host (apache#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 * [KVM] Fix VM migration error due to VNC password on libvirt limiting versions (apache#6404) * [KVM] Fix VM migration error due to VNC password on libvirt limiting versions * Fix passwd value * Simplify implementation Co-authored-by: slavkap <[email protected]> Co-authored-by: Wei Zhou <[email protected]> Co-authored-by: Nicolas Vazquez <[email protected]>

[KVM] Disable IOURING by default on agents

f9aa91c

nvazquez added component:kvm Severity:BLOCKER labels May 19, 2022

nvazquez added this to the 4.17.0.0 milestone May 19, 2022

nvazquez requested review from DaanHoogland, NuxRo, Pearl1594, andrijapanicsb, rohityadavcloud, shwstppr, slavkap and weizhouapache May 19, 2022 11:27

boring-cyborg bot added the component:agent label May 19, 2022

nvazquez requested review from GabrielBrascher and wido May 19, 2022 11:29

weizhouapache approved these changes May 19, 2022

View reviewed changes

GabrielBrascher approved these changes May 19, 2022

View reviewed changes

rohityadavcloud approved these changes May 19, 2022

View reviewed changes

apache deleted a comment from blueorangutan May 20, 2022

DaanHoogland approved these changes May 20, 2022

View reviewed changes

slavkap reviewed May 21, 2022

View reviewed changes

leolleeooleo mentioned this pull request May 21, 2022

KVM NFS disk IO driver supporting IO_URING #5012

Merged

5 tasks

Move method

fa694c2

slavkap reviewed May 23, 2022

View reviewed changes

slavkap approved these changes May 23, 2022

View reviewed changes

Add log

c16e752

nvazquez merged commit dc975df into apache:4.16 May 23, 2022

		String enableIoUringConfig = (String) params.get(ENABLE_IO_URING_PROPERTY);
		enableIoUring = isIoUringEnabled(enableIoUringConfig);

	try {
	_hvVersion = conn.getVersion();
	_hvVersion = _hvVersion % 1000000 / 1000;
	_hypervisorLibvirtVersion = conn.getLibVirVersion();
	_hypervisorQemuVersion = conn.getVersion();
	} catch (final LibvirtException e) {
	s_logger.trace("Ignoring libvirt error.", e);
	}

[KVM] Enable IOURING only when it is available on the host #6399

[KVM] Enable IOURING only when it is available on the host #6399

Uh oh!

Conversation

nvazquez commented May 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented May 19, 2022

Uh oh!

blueorangutan commented May 19, 2022

Uh oh!

GabrielBrascher commented May 19, 2022

Uh oh!

GabrielBrascher left a comment

Choose a reason for hiding this comment

Uh oh!

rohityadavcloud commented May 19, 2022

Uh oh!

blueorangutan commented May 19, 2022

Uh oh!

wido commented May 19, 2022

Uh oh!

GabrielBrascher commented May 19, 2022

Uh oh!

andrijapanicsb commented May 19, 2022

Uh oh!

GabrielBrascher commented May 19, 2022

Uh oh!

rohityadavcloud commented May 19, 2022

Uh oh!

slavkap commented May 19, 2022

Uh oh!

nvazquez commented May 19, 2022

Uh oh!

blueorangutan commented May 20, 2022

Uh oh!

nvazquez commented May 20, 2022

Uh oh!

blueorangutan commented May 20, 2022

Uh oh!

blueorangutan commented May 21, 2022

Uh oh!

slavkap May 21, 2022

Choose a reason for hiding this comment

Uh oh!

nvazquez May 21, 2022

Choose a reason for hiding this comment

Uh oh!

nvazquez commented May 21, 2022

Uh oh!

blueorangutan commented May 21, 2022

Uh oh!

blueorangutan commented May 21, 2022

Uh oh!

nvazquez commented May 21, 2022

Uh oh!

blueorangutan commented May 21, 2022

Uh oh!

blueorangutan commented May 21, 2022

Uh oh!

rohityadavcloud commented May 21, 2022

Uh oh!

blueorangutan commented May 21, 2022

Uh oh!

blueorangutan commented May 21, 2022

Uh oh!

rohityadavcloud commented May 22, 2022

Uh oh!

blueorangutan commented May 22, 2022

Uh oh!

nvazquez commented May 19, 2022 •

edited

Loading