Skip to content

Commit a6cef7a

Browse files
rp-rp-
authored
linstor/kvm: Workaround a qemu bug and IDE bus discard enabled. (apache#9859)
qemu has a bug versions prior 7.0 with discard enabled and using the IDE bus. It would crash the qemu process and kill the virtual machine, this is most noticeable on installing a windows guest from the Windows ISO installer.
1 parent 175eed2 commit a6cef7a

File tree

4 files changed

+134
-3
lines changed

4 files changed

+134
-3
lines changed

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
375375
protected static final String DEFAULT_TUNGSTEN_VIF_DRIVER_CLASS_NAME = "com.cloud.hypervisor.kvm.resource.VRouterVifDriver";
376376
private final static long HYPERVISOR_LIBVIRT_VERSION_SUPPORTS_IO_URING = 6003000;
377377
private final static long HYPERVISOR_QEMU_VERSION_SUPPORTS_IO_URING = 5000000;
378+
private final static long HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED = 7000000;
378379

379380
protected HypervisorType hypervisorType;
380381
protected String hypervisorURI;
@@ -3033,7 +3034,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
30333034
}
30343035
} else if (volume.getType() != Volume.Type.ISO) {
30353036
final PrimaryDataStoreTO store = (PrimaryDataStoreTO)data.getDataStore();
3036-
physicalDisk = storagePoolManager.getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath());
3037+
physicalDisk = getStoragePoolMgr().getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath());
30373038
pool = physicalDisk.getPool();
30383039
}
30393040

@@ -3128,7 +3129,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
31283129
else {
31293130
disk.defBlockBasedDisk(physicalDisk.getPath(), devId, diskBusType);
31303131
}
3131-
if (pool.getType() == StoragePoolType.Linstor) {
3132+
if (pool.getType() == StoragePoolType.Linstor && isQemuDiscardBugFree(diskBusType)) {
31323133
disk.setDiscard(DiscardType.UNMAP);
31333134
}
31343135
} else {
@@ -3275,6 +3276,16 @@ private boolean isIoUringEnabled() {
32753276
return isUbuntuHost() || isIoUringSupportedByQemu();
32763277
}
32773278

3279+
/**
3280+
* Qemu has a bug with discard enabled on IDE bus devices if qemu version < 7.0.
3281+
* <a href="https://bugzilla.redhat.com/show_bug.cgi?id=2029980">redhat bug entry</a>
3282+
* @param diskBus used for the disk
3283+
* @return true if it is safe to enable discard, otherwise false.
3284+
*/
3285+
public boolean isQemuDiscardBugFree(DiskDef.DiskBus diskBus) {
3286+
return diskBus != DiskDef.DiskBus.IDE || getHypervisorQemuVersion() >= HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED;
3287+
}
3288+
32783289
public boolean isUbuntuHost() {
32793290
Map<String, String> versionString = getVersionStrings();
32803291
String hostKey = "Host.OS";

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1444,7 +1444,7 @@ protected synchronized void attachOrDetachDisk(final Connect conn, final boolean
14441444
diskdef.defFileBasedDisk(attachingDisk.getPath(), devId, busT, DiskDef.DiskFmtType.QCOW2);
14451445
} else if (attachingDisk.getFormat() == PhysicalDiskFormat.RAW) {
14461446
diskdef.defBlockBasedDisk(attachingDisk.getPath(), devId, busT);
1447-
if (attachingPool.getType() == StoragePoolType.Linstor) {
1447+
if (attachingPool.getType() == StoragePoolType.Linstor && resource.isQemuDiscardBugFree(busT)) {
14481448
diskdef.setDiscard(DiscardType.UNMAP);
14491449
}
14501450
}

‎plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,12 @@
5858

5959
import com.cloud.utils.net.NetUtils;
6060

61+
import com.cloud.vm.VmDetailConstants;
6162
import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy;
6263
import org.apache.cloudstack.storage.command.AttachAnswer;
6364
import org.apache.cloudstack.storage.command.AttachCommand;
65+
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
66+
import org.apache.cloudstack.storage.to.VolumeObjectTO;
6467
import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
6568
import org.apache.cloudstack.utils.linux.CPUStat;
6669
import org.apache.cloudstack.utils.linux.MemStat;
@@ -6295,4 +6298,114 @@ public void setMaxHostCpuSharesIfCGroupV2TestShouldNotCalculateMaxCpuCapacityIfH
62956298
Assert.assertEquals(expectedShares, libvirtComputingResourceSpy.getHostCpuMaxCapacity());
62966299
}
62976300
}
6301+
6302+
@Test
6303+
public void createLinstorVdb() throws LibvirtException, InternalErrorException, URISyntaxException {
6304+
final Connect connect = Mockito.mock(Connect.class);
6305+
6306+
final int id = random.nextInt(65534);
6307+
final String name = "test-instance-1";
6308+
6309+
final int cpus = 2;
6310+
final int speed = 1024;
6311+
final int minRam = 256 * 1024;
6312+
final int maxRam = 512 * 1024;
6313+
final String os = "Ubuntu";
6314+
final String vncPassword = "mySuperSecretPassword";
6315+
6316+
final VirtualMachineTO to = new VirtualMachineTO(id, name, VirtualMachine.Type.User, cpus, speed, minRam,
6317+
maxRam, BootloaderType.HVM, os, false, false, vncPassword);
6318+
to.setVncAddr("");
6319+
to.setArch("x86_64");
6320+
to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9");
6321+
to.setVcpuMaxLimit(cpus + 1);
6322+
final HashMap<String, String> vmToDetails = new HashMap<>();
6323+
to.setDetails(vmToDetails);
6324+
6325+
String diskLinPath = "9ebe53c1-3d35-46e5-b7aa-6fc223ba0fcf";
6326+
final DiskTO diskTO = new DiskTO();
6327+
diskTO.setDiskSeq(1L);
6328+
diskTO.setType(Volume.Type.ROOT);
6329+
diskTO.setDetails(new HashMap<>());
6330+
diskTO.setPath(diskLinPath);
6331+
6332+
final PrimaryDataStoreTO primaryDataStoreTO = Mockito.mock(PrimaryDataStoreTO.class);
6333+
String pDSTOUUID = "9ebe53c1-3d35-46e5-b7aa-6fc223ac4fcf";
6334+
when(primaryDataStoreTO.getPoolType()).thenReturn(StoragePoolType.Linstor);
6335+
when(primaryDataStoreTO.getUuid()).thenReturn(pDSTOUUID);
6336+
6337+
VolumeObjectTO dataTO = new VolumeObjectTO();
6338+
6339+
dataTO.setUuid("12be53c1-3d35-46e5-b7aa-6fc223ba0f34");
6340+
dataTO.setPath(diskTO.getPath());
6341+
dataTO.setDataStore(primaryDataStoreTO);
6342+
diskTO.setData(dataTO);
6343+
to.setDisks(new DiskTO[]{diskTO});
6344+
6345+
String path = "/dev/drbd1020";
6346+
final KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class);
6347+
final KVMStoragePool storagePool = Mockito.mock(KVMStoragePool.class);
6348+
final KVMPhysicalDisk vol = Mockito.mock(KVMPhysicalDisk.class);
6349+
6350+
when(libvirtComputingResourceSpy.getStoragePoolMgr()).thenReturn(storagePoolMgr);
6351+
when(storagePool.getType()).thenReturn(StoragePoolType.Linstor);
6352+
when(storagePoolMgr.getPhysicalDisk(StoragePoolType.Linstor, pDSTOUUID, diskLinPath)).thenReturn(vol);
6353+
when(vol.getPath()).thenReturn(path);
6354+
when(vol.getPool()).thenReturn(storagePool);
6355+
when(vol.getFormat()).thenReturn(PhysicalDiskFormat.RAW);
6356+
6357+
// 1. test Bus: IDE and broken qemu version -> NO discard
6358+
when(libvirtComputingResourceSpy.getHypervisorQemuVersion()).thenReturn(6000000L);
6359+
vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.IDE.name());
6360+
{
6361+
LibvirtVMDef vm = new LibvirtVMDef();
6362+
vm.addComp(new DevicesDef());
6363+
libvirtComputingResourceSpy.createVbd(connect, to, name, vm);
6364+
6365+
DiskDef rootDisk = vm.getDevices().getDisks().get(0);
6366+
assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType());
6367+
assertEquals(DiskDef.DiskBus.IDE, rootDisk.getBusType());
6368+
assertEquals(DiskDef.DiscardType.IGNORE, rootDisk.getDiscard());
6369+
}
6370+
6371+
// 2. test Bus: VIRTIO and broken qemu version -> discard unmap
6372+
vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.VIRTIO.name());
6373+
{
6374+
LibvirtVMDef vm = new LibvirtVMDef();
6375+
vm.addComp(new DevicesDef());
6376+
libvirtComputingResourceSpy.createVbd(connect, to, name, vm);
6377+
6378+
DiskDef rootDisk = vm.getDevices().getDisks().get(0);
6379+
assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType());
6380+
assertEquals(DiskDef.DiskBus.VIRTIO, rootDisk.getBusType());
6381+
assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard());
6382+
}
6383+
6384+
// 3. test Bus; IDE and "good" qemu version -> discard unmap
6385+
vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.IDE.name());
6386+
when(libvirtComputingResourceSpy.getHypervisorQemuVersion()).thenReturn(7000000L);
6387+
{
6388+
LibvirtVMDef vm = new LibvirtVMDef();
6389+
vm.addComp(new DevicesDef());
6390+
libvirtComputingResourceSpy.createVbd(connect, to, name, vm);
6391+
6392+
DiskDef rootDisk = vm.getDevices().getDisks().get(0);
6393+
assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType());
6394+
assertEquals(DiskDef.DiskBus.IDE, rootDisk.getBusType());
6395+
assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard());
6396+
}
6397+
6398+
// 4. test Bus: VIRTIO and "good" qemu version -> discard unmap
6399+
vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.VIRTIO.name());
6400+
{
6401+
LibvirtVMDef vm = new LibvirtVMDef();
6402+
vm.addComp(new DevicesDef());
6403+
libvirtComputingResourceSpy.createVbd(connect, to, name, vm);
6404+
6405+
DiskDef rootDisk = vm.getDevices().getDisks().get(0);
6406+
assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType());
6407+
assertEquals(DiskDef.DiskBus.VIRTIO, rootDisk.getBusType());
6408+
assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard());
6409+
}
6410+
}
62986411
}

‎plugins/storage/volume/linstor/CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ All notable changes to Linstor CloudStack plugin will be documented in this file
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [2024-10-28]
9+
10+
### Fixed
11+
12+
- Disable discard="unmap" for ide devices and qemu < 7.0
13+
https://bugzilla.redhat.com/show_bug.cgi?id=2029980
14+
815
## [2024-10-04]
916

1017
### Added

0 commit comments

Comments
 (0)