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 fc1c30a5988f..43923059d912 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 @@ -375,6 +375,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv protected static final String DEFAULT_TUNGSTEN_VIF_DRIVER_CLASS_NAME = "com.cloud.hypervisor.kvm.resource.VRouterVifDriver"; private final static long HYPERVISOR_LIBVIRT_VERSION_SUPPORTS_IO_URING = 6003000; private final static long HYPERVISOR_QEMU_VERSION_SUPPORTS_IO_URING = 5000000; + private final static long HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED = 7000000; protected HypervisorType hypervisorType; protected String hypervisorURI; @@ -3033,7 +3034,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { } } else if (volume.getType() != Volume.Type.ISO) { final PrimaryDataStoreTO store = (PrimaryDataStoreTO)data.getDataStore(); - physicalDisk = storagePoolManager.getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath()); + physicalDisk = getStoragePoolMgr().getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath()); pool = physicalDisk.getPool(); } @@ -3128,7 +3129,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { else { disk.defBlockBasedDisk(physicalDisk.getPath(), devId, diskBusType); } - if (pool.getType() == StoragePoolType.Linstor) { + if (pool.getType() == StoragePoolType.Linstor && isQemuDiscardBugFree(diskBusType)) { disk.setDiscard(DiscardType.UNMAP); } } else { @@ -3275,6 +3276,16 @@ private boolean isIoUringEnabled() { return isUbuntuHost() || isIoUringSupportedByQemu(); } + /** + * Qemu has a bug with discard enabled on IDE bus devices if qemu version < 7.0. + * redhat bug entry + * @param diskBus used for the disk + * @return true if it is safe to enable discard, otherwise false. + */ + public boolean isQemuDiscardBugFree(DiskDef.DiskBus diskBus) { + return diskBus != DiskDef.DiskBus.IDE || getHypervisorQemuVersion() >= HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED; + } + public boolean isUbuntuHost() { Map versionString = getVersionStrings(); String hostKey = "Host.OS"; 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 e3ee131a84b2..6ef2faaab1c4 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 @@ -1444,7 +1444,7 @@ protected synchronized void attachOrDetachDisk(final Connect conn, final boolean diskdef.defFileBasedDisk(attachingDisk.getPath(), devId, busT, DiskDef.DiskFmtType.QCOW2); } else if (attachingDisk.getFormat() == PhysicalDiskFormat.RAW) { diskdef.defBlockBasedDisk(attachingDisk.getPath(), devId, busT); - if (attachingPool.getType() == StoragePoolType.Linstor) { + if (attachingPool.getType() == StoragePoolType.Linstor && resource.isQemuDiscardBugFree(busT)) { diskdef.setDiscard(DiscardType.UNMAP); } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index aac7f7343afe..17bff8325ce4 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -58,9 +58,12 @@ import com.cloud.utils.net.NetUtils; +import com.cloud.vm.VmDetailConstants; import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy; import org.apache.cloudstack.storage.command.AttachAnswer; import org.apache.cloudstack.storage.command.AttachCommand; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; import org.apache.cloudstack.utils.linux.CPUStat; import org.apache.cloudstack.utils.linux.MemStat; @@ -6295,4 +6298,114 @@ public void setMaxHostCpuSharesIfCGroupV2TestShouldNotCalculateMaxCpuCapacityIfH Assert.assertEquals(expectedShares, libvirtComputingResourceSpy.getHostCpuMaxCapacity()); } } + + @Test + public void createLinstorVdb() throws LibvirtException, InternalErrorException, URISyntaxException { + final Connect connect = Mockito.mock(Connect.class); + + final int id = random.nextInt(65534); + final String name = "test-instance-1"; + + final int cpus = 2; + final int speed = 1024; + final int minRam = 256 * 1024; + final int maxRam = 512 * 1024; + final String os = "Ubuntu"; + final String vncPassword = "mySuperSecretPassword"; + + final VirtualMachineTO to = new VirtualMachineTO(id, name, VirtualMachine.Type.User, cpus, speed, minRam, + maxRam, BootloaderType.HVM, os, false, false, vncPassword); + to.setVncAddr(""); + to.setArch("x86_64"); + to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9"); + to.setVcpuMaxLimit(cpus + 1); + final HashMap vmToDetails = new HashMap<>(); + to.setDetails(vmToDetails); + + String diskLinPath = "9ebe53c1-3d35-46e5-b7aa-6fc223ba0fcf"; + final DiskTO diskTO = new DiskTO(); + diskTO.setDiskSeq(1L); + diskTO.setType(Volume.Type.ROOT); + diskTO.setDetails(new HashMap<>()); + diskTO.setPath(diskLinPath); + + final PrimaryDataStoreTO primaryDataStoreTO = Mockito.mock(PrimaryDataStoreTO.class); + String pDSTOUUID = "9ebe53c1-3d35-46e5-b7aa-6fc223ac4fcf"; + when(primaryDataStoreTO.getPoolType()).thenReturn(StoragePoolType.Linstor); + when(primaryDataStoreTO.getUuid()).thenReturn(pDSTOUUID); + + VolumeObjectTO dataTO = new VolumeObjectTO(); + + dataTO.setUuid("12be53c1-3d35-46e5-b7aa-6fc223ba0f34"); + dataTO.setPath(diskTO.getPath()); + dataTO.setDataStore(primaryDataStoreTO); + diskTO.setData(dataTO); + to.setDisks(new DiskTO[]{diskTO}); + + String path = "/dev/drbd1020"; + final KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + final KVMStoragePool storagePool = Mockito.mock(KVMStoragePool.class); + final KVMPhysicalDisk vol = Mockito.mock(KVMPhysicalDisk.class); + + when(libvirtComputingResourceSpy.getStoragePoolMgr()).thenReturn(storagePoolMgr); + when(storagePool.getType()).thenReturn(StoragePoolType.Linstor); + when(storagePoolMgr.getPhysicalDisk(StoragePoolType.Linstor, pDSTOUUID, diskLinPath)).thenReturn(vol); + when(vol.getPath()).thenReturn(path); + when(vol.getPool()).thenReturn(storagePool); + when(vol.getFormat()).thenReturn(PhysicalDiskFormat.RAW); + + // 1. test Bus: IDE and broken qemu version -> NO discard + when(libvirtComputingResourceSpy.getHypervisorQemuVersion()).thenReturn(6000000L); + vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.IDE.name()); + { + LibvirtVMDef vm = new LibvirtVMDef(); + vm.addComp(new DevicesDef()); + libvirtComputingResourceSpy.createVbd(connect, to, name, vm); + + DiskDef rootDisk = vm.getDevices().getDisks().get(0); + assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType()); + assertEquals(DiskDef.DiskBus.IDE, rootDisk.getBusType()); + assertEquals(DiskDef.DiscardType.IGNORE, rootDisk.getDiscard()); + } + + // 2. test Bus: VIRTIO and broken qemu version -> discard unmap + vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.VIRTIO.name()); + { + LibvirtVMDef vm = new LibvirtVMDef(); + vm.addComp(new DevicesDef()); + libvirtComputingResourceSpy.createVbd(connect, to, name, vm); + + DiskDef rootDisk = vm.getDevices().getDisks().get(0); + assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType()); + assertEquals(DiskDef.DiskBus.VIRTIO, rootDisk.getBusType()); + assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard()); + } + + // 3. test Bus; IDE and "good" qemu version -> discard unmap + vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.IDE.name()); + when(libvirtComputingResourceSpy.getHypervisorQemuVersion()).thenReturn(7000000L); + { + LibvirtVMDef vm = new LibvirtVMDef(); + vm.addComp(new DevicesDef()); + libvirtComputingResourceSpy.createVbd(connect, to, name, vm); + + DiskDef rootDisk = vm.getDevices().getDisks().get(0); + assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType()); + assertEquals(DiskDef.DiskBus.IDE, rootDisk.getBusType()); + assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard()); + } + + // 4. test Bus: VIRTIO and "good" qemu version -> discard unmap + vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.VIRTIO.name()); + { + LibvirtVMDef vm = new LibvirtVMDef(); + vm.addComp(new DevicesDef()); + libvirtComputingResourceSpy.createVbd(connect, to, name, vm); + + DiskDef rootDisk = vm.getDevices().getDisks().get(0); + assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType()); + assertEquals(DiskDef.DiskBus.VIRTIO, rootDisk.getBusType()); + assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard()); + } + } } diff --git a/plugins/storage/volume/linstor/CHANGELOG.md b/plugins/storage/volume/linstor/CHANGELOG.md index a69cdf5aa31b..a0a53d201196 100644 --- a/plugins/storage/volume/linstor/CHANGELOG.md +++ b/plugins/storage/volume/linstor/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to Linstor CloudStack plugin will be documented in this file The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2024-10-28] + +### Fixed + +- Disable discard="unmap" for ide devices and qemu < 7.0 + https://bugzilla.redhat.com/show_bug.cgi?id=2029980 + ## [2024-10-04] ### Added