From 99ca62756c2eb2a880962b12ff965e9d21eea08c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 16 Jan 2026 18:53:24 +0530 Subject: [PATCH 1/3] kvm: honour data disk controller for vm during attach volume Fixes #12090 Some years back #4569 allowed honouring dataDiskController for KVM VMs during start but the same behviour is not seen while attaching a volume to a running VM. This PR first checks in the AttachCommand if a valid controller is present. If a valid controller/bus type is not present than existing logic is used. Signed-off-by: Abhishek Kumar --- .../resource/LibvirtComputingResource.java | 11 +- .../hypervisor/kvm/resource/LibvirtVMDef.java | 9 ++ .../kvm/storage/KVMStorageProcessor.java | 123 ++++++++++-------- 3 files changed, 84 insertions(+), 59 deletions(-) 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 b66a838a3a5..c7b2747a777 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 @@ -4371,12 +4371,11 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv String dataDiskController = details.get(VmDetailConstants.DATA_DISK_CONTROLLER); if (StringUtils.isNotBlank(dataDiskController)) { - LOGGER.debug("Passed custom disk controller for DATA disk " + dataDiskController); - for (DiskDef.DiskBus bus : DiskDef.DiskBus.values()) { - if (bus.toString().equalsIgnoreCase(dataDiskController)) { - LOGGER.debug("Found matching enum for disk controller for DATA disk " + dataDiskController); - return bus; - } + LOGGER.debug("Passed custom disk controller for DATA disk {}", dataDiskController); + DiskDef.DiskBus bus = DiskDef.DiskBus.fromValue(dataDiskController); + if (bus != null) { + LOGGER.debug("Found matching enum for disk controller for DATA disk {}", dataDiskController); + return bus; } } return null; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java index a38e7a02357..696e71bea80 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java @@ -686,6 +686,15 @@ public class LibvirtVMDef { _bus = bus; } + public static DiskBus fromValue(String bus) { + for (DiskBus b : DiskBus.values()) { + if (b.toString().equalsIgnoreCase(bus)) { + return b; + } + } + return null; + } + @Override public String toString() { return _bus; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 33bd41ee6ba..ac6fb6d1c34 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1320,26 +1320,28 @@ public class KVMStorageProcessor implements StorageProcessor { /** * Attaches or detaches a disk to an instance. - * @param conn libvirt connection - * @param attach boolean that determines whether the device will be attached or detached - * @param vmName instance name - * @param attachingDisk kvm physical disk - * @param devId device id in instance + * + * @param conn libvirt connection + * @param attach boolean that determines whether the device will be attached or detached + * @param vmName instance name + * @param attachingDisk kvm physical disk + * @param devId device id in instance * @param serial - * @param bytesReadRate bytes read rate - * @param bytesReadRateMax bytes read rate max - * @param bytesReadRateMaxLength bytes read rate max length - * @param bytesWriteRate bytes write rate - * @param bytesWriteRateMax bytes write rate amx + * @param bytesReadRate bytes read rate + * @param bytesReadRateMax bytes read rate max + * @param bytesReadRateMaxLength bytes read rate max length + * @param bytesWriteRate bytes write rate + * @param bytesWriteRateMax bytes write rate amx * @param bytesWriteRateMaxLength bytes write rate max length - * @param iopsReadRate iops read rate - * @param iopsReadRateMax iops read rate max - * @param iopsReadRateMaxLength iops read rate max length - * @param iopsWriteRate iops write rate - * @param iopsWriteRateMax iops write rate max - * @param iopsWriteRateMaxLength iops write rate max length - * @param cacheMode cache mode - * @param encryptDetails encrypt details + * @param iopsReadRate iops read rate + * @param iopsReadRateMax iops read rate max + * @param iopsReadRateMaxLength iops read rate max length + * @param iopsWriteRate iops write rate + * @param iopsWriteRateMax iops write rate max + * @param iopsWriteRateMaxLength iops write rate max length + * @param cacheMode cache mode + * @param encryptDetails encrypt details + * @param controllerInfo * @throws LibvirtException * @throws InternalErrorException */ @@ -1347,37 +1349,39 @@ public class KVMStorageProcessor implements StorageProcessor { final String serial, final Long bytesReadRate, final Long bytesReadRateMax, final Long bytesReadRateMaxLength, final Long bytesWriteRate, final Long bytesWriteRateMax, final Long bytesWriteRateMaxLength, final Long iopsReadRate, final Long iopsReadRateMax, final Long iopsReadRateMaxLength, final Long iopsWriteRate, final Long iopsWriteRateMax, - final Long iopsWriteRateMaxLength, final String cacheMode, final DiskDef.LibvirtDiskEncryptDetails encryptDetails, Map details) + final Long iopsWriteRateMaxLength, final String cacheMode, final DiskDef.LibvirtDiskEncryptDetails encryptDetails, Map details, Map controllerInfo) throws LibvirtException, InternalErrorException { attachOrDetachDisk(conn, attach, vmName, attachingDisk, devId, serial, bytesReadRate, bytesReadRateMax, bytesReadRateMaxLength, bytesWriteRate, bytesWriteRateMax, bytesWriteRateMaxLength, iopsReadRate, iopsReadRateMax, iopsReadRateMaxLength, iopsWriteRate, - iopsWriteRateMax, iopsWriteRateMaxLength, cacheMode, encryptDetails, 0l, details); + iopsWriteRateMax, iopsWriteRateMaxLength, cacheMode, encryptDetails, 0l, details, controllerInfo); } /** * * Attaches or detaches a disk to an instance. - * @param conn libvirt connection - * @param attach boolean that determines whether the device will be attached or detached - * @param vmName instance name - * @param attachingDisk kvm physical disk - * @param devId device id in instance + * + * @param conn libvirt connection + * @param attach boolean that determines whether the device will be attached or detached + * @param vmName instance name + * @param attachingDisk kvm physical disk + * @param devId device id in instance * @param serial - * @param bytesReadRate bytes read rate - * @param bytesReadRateMax bytes read rate max - * @param bytesReadRateMaxLength bytes read rate max length - * @param bytesWriteRate bytes write rate - * @param bytesWriteRateMax bytes write rate amx + * @param bytesReadRate bytes read rate + * @param bytesReadRateMax bytes read rate max + * @param bytesReadRateMaxLength bytes read rate max length + * @param bytesWriteRate bytes write rate + * @param bytesWriteRateMax bytes write rate amx * @param bytesWriteRateMaxLength bytes write rate max length - * @param iopsReadRate iops read rate - * @param iopsReadRateMax iops read rate max - * @param iopsReadRateMaxLength iops read rate max length - * @param iopsWriteRate iops write rate - * @param iopsWriteRateMax iops write rate max - * @param iopsWriteRateMaxLength iops write rate max length - * @param cacheMode cache mode - * @param encryptDetails encrypt details - * @param waitDetachDevice value set in milliseconds to wait before assuming device removal failed + * @param iopsReadRate iops read rate + * @param iopsReadRateMax iops read rate max + * @param iopsReadRateMaxLength iops read rate max length + * @param iopsWriteRate iops write rate + * @param iopsWriteRateMax iops write rate max + * @param iopsWriteRateMaxLength iops write rate max length + * @param cacheMode cache mode + * @param encryptDetails encrypt details + * @param waitDetachDevice value set in milliseconds to wait before assuming device removal failed + * @param controllerInfo * @throws LibvirtException * @throws InternalErrorException */ @@ -1386,7 +1390,7 @@ public class KVMStorageProcessor implements StorageProcessor { final Long bytesWriteRate, final Long bytesWriteRateMax, final Long bytesWriteRateMaxLength, final Long iopsReadRate, final Long iopsReadRateMax, final Long iopsReadRateMaxLength, final Long iopsWriteRate, final Long iopsWriteRateMax, final Long iopsWriteRateMaxLength, final String cacheMode, final DiskDef.LibvirtDiskEncryptDetails encryptDetails, - long waitDetachDevice, Map details) + long waitDetachDevice, Map details, Map controllerInfo) throws LibvirtException, InternalErrorException { List disks = null; @@ -1423,17 +1427,7 @@ public class KVMStorageProcessor implements StorageProcessor { return; } } else { - DiskDef.DiskBus busT = DiskDef.DiskBus.VIRTIO; - for (final DiskDef disk : disks) { - if (disk.getDeviceType() == DeviceType.DISK) { - if (disk.getBusType() == DiskDef.DiskBus.SCSI) { - busT = DiskDef.DiskBus.SCSI; - } else if (disk.getBusType() == DiskDef.DiskBus.VIRTIOBLK) { - busT = DiskDef.DiskBus.VIRTIOBLK; - } - break; - } - } + DiskDef.DiskBus busT = getAttachDiskBusType(devId, disks, controllerInfo); diskdef = new DiskDef(); if (busT == DiskDef.DiskBus.SCSI || busT == DiskDef.DiskBus.VIRTIOBLK) { diskdef.setQemuDriver(true); @@ -1538,6 +1532,28 @@ public class KVMStorageProcessor implements StorageProcessor { } } + protected DiskDef.DiskBus getAttachDiskBusType(int deviceId, List disks, Map controllerInfo) { + String controllerKey = deviceId == 0 ? VmDetailConstants.ROOT_DISK_CONTROLLER : VmDetailConstants.DATA_DISK_CONTROLLER; + String diskController = MapUtils.getString(controllerInfo, controllerKey); + DiskDef.DiskBus busType = DiskDef.DiskBus.fromValue(diskController); + if (diskController != null) { + logger.debug("Using controller '{}' from command specified as {} while attaching disk (deviceId={})", + diskController, controllerKey, deviceId); + return busType; + } + for (final DiskDef disk : disks) { + if (disk.getDeviceType() != DeviceType.DISK) { + continue; + } + if (disk.getBusType() == DiskDef.DiskBus.SCSI) { + return DiskDef.DiskBus.SCSI; + } else if (disk.getBusType() == DiskDef.DiskBus.VIRTIOBLK) { + return DiskDef.DiskBus.VIRTIOBLK; + } + } + return DiskDef.DiskBus.VIRTIO; + } + @Override public Answer attachVolume(final AttachCommand cmd) { final DiskTO disk = cmd.getDisk(); @@ -1565,7 +1581,8 @@ public class KVMStorageProcessor implements StorageProcessor { vol.getBytesReadRate(), vol.getBytesReadRateMax(), vol.getBytesReadRateMaxLength(), vol.getBytesWriteRate(), vol.getBytesWriteRateMax(), vol.getBytesWriteRateMaxLength(), vol.getIopsReadRate(), vol.getIopsReadRateMax(), vol.getIopsReadRateMaxLength(), - vol.getIopsWriteRate(), vol.getIopsWriteRateMax(), vol.getIopsWriteRateMaxLength(), volCacheMode, encryptDetails, disk.getDetails()); + vol.getIopsWriteRate(), vol.getIopsWriteRateMax(), vol.getIopsWriteRateMaxLength(), volCacheMode, + encryptDetails, disk.getDetails(), cmd.getControllerInfo()); return new AttachAnswer(disk); } catch (final LibvirtException e) { @@ -1602,7 +1619,7 @@ public class KVMStorageProcessor implements StorageProcessor { vol.getBytesReadRate(), vol.getBytesReadRateMax(), vol.getBytesReadRateMaxLength(), vol.getBytesWriteRate(), vol.getBytesWriteRateMax(), vol.getBytesWriteRateMaxLength(), vol.getIopsReadRate(), vol.getIopsReadRateMax(), vol.getIopsReadRateMaxLength(), - vol.getIopsWriteRate(), vol.getIopsWriteRateMax(), vol.getIopsWriteRateMaxLength(), volCacheMode, null, waitDetachDevice, null); + vol.getIopsWriteRate(), vol.getIopsWriteRateMax(), vol.getIopsWriteRateMaxLength(), volCacheMode, null, waitDetachDevice, null, null); storagePoolMgr.disconnectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), vol.getPath()); From e8ab2aa0f868f0ce1c2fc542281084b7262c3106 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 20 Jan 2026 09:36:17 +0530 Subject: [PATCH 2/3] Apply suggestions from code review --- .../com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index ac6fb6d1c34..a2acd7e72f2 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1359,7 +1359,6 @@ public class KVMStorageProcessor implements StorageProcessor { /** * * Attaches or detaches a disk to an instance. - * * @param conn libvirt connection * @param attach boolean that determines whether the device will be attached or detached * @param vmName instance name From 5dec11fc0b06ae92d8b13412b1793b2775825d4e Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 20 Jan 2026 09:36:58 +0530 Subject: [PATCH 3/3] Apply suggestion from @shwstppr --- .../com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index a2acd7e72f2..87ca531bb74 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1320,7 +1320,6 @@ public class KVMStorageProcessor implements StorageProcessor { /** * Attaches or detaches a disk to an instance. - * * @param conn libvirt connection * @param attach boolean that determines whether the device will be attached or detached * @param vmName instance name