Skip to content
Merged
2 changes: 1 addition & 1 deletion agent/conf/agent.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,6 @@ public class AgentProperties{
*/
public static final Property<Boolean> ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM = new Property<Boolean>("enable.manually.setting.cpu.topology.on.kvm.vm", true);

/**
* Enable manually IO driver on KVM's VM. <br>
* Data type: boolean.<br>
* Default value: true.
*/
public static final Property<Boolean> ENABLE_IO_URING = new Property<Boolean>("enable.io.uring", true);

Copy link
Contributor

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_uring even if it is supported

Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks @slavkap @NuxRo

public static class Property <T>{
private final String name;
private final T defaultValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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"));

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please, let's do the proper logging.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this reliable @nvazquez cc others?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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("/");
Expand Down Expand Up @@ -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 {
Expand Down