Skip to content
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.

Sign up for GitHub

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

Jump to bottom

linstor/kvm: Workaround a qemu bug and IDE bus discard enabled. #9859

Merged
merged 1 commit into from
Nov 4, 2024
Merged

linstor/kvm: Workaround a qemu bug and IDE bus discard enabled. #9859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions ...rvisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
* <a href="https://bugzilla.redhat.com/show_bug.cgi?id=2029980">redhat bug entry</a>
* @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<String, String> versionString = getVersionStrings();
String hostKey = "Host.OS";
Expand Down
2 changes: 1 addition & 1 deletion ...s/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
113 changes: 113 additions & 0 deletions ...ors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, String> 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());
}
}
}
7 changes: 7 additions & 0 deletions plugins/storage/volume/linstor/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading