Skip to content

Commit 1d5eebb

Browse files
JoaoJandreJoaoJandre
committed
Merge remote-tracking branch 'origin/main' into differential-snapshots
2 parents 885d2a4 + f7b7339 commit 1d5eebb

File tree

16 files changed

+275
-57
lines changed

16 files changed

+275
-57
lines changed

‎engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2078,8 +2078,8 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
20782078
migrateDiskInfo = configureMigrateDiskInfo(srcVolumeInfo, destPath, backingPath);
20792079
migrateDiskInfo.setSourceDiskOnStorageFileSystem(isStoragePoolTypeOfFile(sourceStoragePool));
20802080
migrateDiskInfoList.add(migrateDiskInfo);
2081-
prepareDiskWithSecretConsumerDetail(vmTO, srcVolumeInfo, destVolumeInfo.getPath());
20822081
}
2082+
prepareDiskWithSecretConsumerDetail(vmTO, srcVolumeInfo, destVolumeInfo.getPath());
20832083

20842084
migrateStorage.put(srcVolumeInfo.getPath(), migrateDiskInfo);
20852085

@@ -2479,7 +2479,8 @@ protected void verifyLiveMigrationForKVM(Map<VolumeInfo, DataStore> volumeDataSt
24792479
throw new CloudRuntimeException("Destination storage pool with ID " + dataStore.getId() + " was not located.");
24802480
}
24812481

2482-
if (srcStoragePoolVO.isManaged() && srcStoragePoolVO.getId() != destStoragePoolVO.getId()) {
2482+
boolean isSrcAndDestPoolPowerFlexStorage = srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex) && destStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex);
2483+
if (srcStoragePoolVO.isManaged() && !isSrcAndDestPoolPowerFlexStorage && srcStoragePoolVO.getId() != destStoragePoolVO.getId()) {
24832484
throw new CloudRuntimeException("Migrating a volume online with KVM from managed storage is not currently supported.");
24842485
}
24852486

‎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
@@ -397,6 +397,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
397397
protected static final String DEFAULT_TUNGSTEN_VIF_DRIVER_CLASS_NAME = "com.cloud.hypervisor.kvm.resource.VRouterVifDriver";
398398
private final static long HYPERVISOR_LIBVIRT_VERSION_SUPPORTS_IO_URING = 6003000;
399399
private final static long HYPERVISOR_QEMU_VERSION_SUPPORTS_IO_URING = 5000000;
400+
private final static long HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED = 7000000;
400401

401402
private static final int MINIMUM_LIBVIRT_VERSION_FOR_INCREMENTAL_SNAPSHOT = 7006000;
402403

@@ -3074,7 +3075,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
30743075
}
30753076
} else if (volume.getType() != Volume.Type.ISO) {
30763077
final PrimaryDataStoreTO store = (PrimaryDataStoreTO)data.getDataStore();
3077-
physicalDisk = storagePoolManager.getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath());
3078+
physicalDisk = getStoragePoolMgr().getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath());
30783079
pool = physicalDisk.getPool();
30793080
}
30803081

@@ -3176,7 +3177,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
31763177
else {
31773178
disk.defBlockBasedDisk(physicalDisk.getPath(), devId, diskBusType);
31783179
}
3179-
if (pool.getType() == StoragePoolType.Linstor) {
3180+
if (pool.getType() == StoragePoolType.Linstor && isQemuDiscardBugFree(diskBusType)) {
31803181
disk.setDiscard(DiscardType.UNMAP);
31813182
}
31823183
} else {
@@ -3323,6 +3324,16 @@ private boolean isIoUringEnabled() {
33233324
return isUbuntuHost() || isIoUringSupportedByQemu();
33243325
}
33253326

3327+
/**
3328+
* Qemu has a bug with discard enabled on IDE bus devices if qemu version < 7.0.
3329+
* <a href="https://bugzilla.redhat.com/show_bug.cgi?id=2029980">redhat bug entry</a>
3330+
* @param diskBus used for the disk
3331+
* @return true if it is safe to enable discard, otherwise false.
3332+
*/
3333+
public boolean isQemuDiscardBugFree(DiskDef.DiskBus diskBus) {
3334+
return diskBus != DiskDef.DiskBus.IDE || getHypervisorQemuVersion() >= HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED;
3335+
}
3336+
33263337
public boolean isUbuntuHost() {
33273338
Map<String, String> versionString = getVersionStrings();
33283339
String hostKey = "Host.OS";

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public Ternary<String, String, String> getInterfaceDetails(String interfaceName)
8989

9090
@Override
9191
public Answer execute(final OvsFetchInterfaceCommand command, final LibvirtComputingResource libvirtComputingResource) {
92-
final String label = "'" + command.getLabel() + "'";
92+
final String label = command.getLabel();
9393

9494
logger.debug("Will look for network with name-label:" + label);
9595
try {

‎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
@@ -1511,7 +1511,7 @@ protected synchronized void attachOrDetachDisk(final Connect conn, final boolean
15111511
diskdef.defFileBasedDisk(attachingDisk.getPath(), devId, busT, DiskDef.DiskFmtType.QCOW2);
15121512
} else if (attachingDisk.getFormat() == PhysicalDiskFormat.RAW) {
15131513
diskdef.defBlockBasedDisk(attachingDisk.getPath(), devId, busT);
1514-
if (attachingPool.getType() == StoragePoolType.Linstor) {
1514+
if (attachingPool.getType() == StoragePoolType.Linstor && resource.isQemuDiscardBugFree(busT)) {
15151515
diskdef.setDiscard(DiscardType.UNMAP);
15161516
}
15171517
}

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

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,11 @@
5959

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

62+
import com.cloud.vm.VmDetailConstants;
6263
import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy;
6364
import org.apache.cloudstack.storage.command.AttachAnswer;
6465
import org.apache.cloudstack.storage.command.AttachCommand;
66+
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
6567
import org.apache.cloudstack.storage.to.VolumeObjectTO;
6668
import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
6769
import org.apache.cloudstack.utils.linux.CPUStat;
@@ -6429,6 +6431,116 @@ public void calculateVmMetricsTestOldStatsIsNotNullCalculatesUtilization() throw
64296431
Assert.assertEquals(newStats.getDiskWriteKBs() - oldStats.getDiskWriteKBs(), metrics.getDiskWriteKBs(), 0);
64306432
}
64316433

6434+
@Test
6435+
public void createLinstorVdb() throws LibvirtException, InternalErrorException, URISyntaxException {
6436+
final Connect connect = Mockito.mock(Connect.class);
6437+
6438+
final int id = random.nextInt(65534);
6439+
final String name = "test-instance-1";
6440+
6441+
final int cpus = 2;
6442+
final int speed = 1024;
6443+
final int minRam = 256 * 1024;
6444+
final int maxRam = 512 * 1024;
6445+
final String os = "Ubuntu";
6446+
final String vncPassword = "mySuperSecretPassword";
6447+
6448+
final VirtualMachineTO to = new VirtualMachineTO(id, name, VirtualMachine.Type.User, cpus, speed, minRam,
6449+
maxRam, BootloaderType.HVM, os, false, false, vncPassword);
6450+
to.setVncAddr("");
6451+
to.setArch("x86_64");
6452+
to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9");
6453+
to.setVcpuMaxLimit(cpus + 1);
6454+
final HashMap<String, String> vmToDetails = new HashMap<>();
6455+
to.setDetails(vmToDetails);
6456+
6457+
String diskLinPath = "9ebe53c1-3d35-46e5-b7aa-6fc223ba0fcf";
6458+
final DiskTO diskTO = new DiskTO();
6459+
diskTO.setDiskSeq(1L);
6460+
diskTO.setType(Volume.Type.ROOT);
6461+
diskTO.setDetails(new HashMap<>());
6462+
diskTO.setPath(diskLinPath);
6463+
6464+
final PrimaryDataStoreTO primaryDataStoreTO = Mockito.mock(PrimaryDataStoreTO.class);
6465+
String pDSTOUUID = "9ebe53c1-3d35-46e5-b7aa-6fc223ac4fcf";
6466+
when(primaryDataStoreTO.getPoolType()).thenReturn(StoragePoolType.Linstor);
6467+
when(primaryDataStoreTO.getUuid()).thenReturn(pDSTOUUID);
6468+
6469+
VolumeObjectTO dataTO = new VolumeObjectTO();
6470+
6471+
dataTO.setUuid("12be53c1-3d35-46e5-b7aa-6fc223ba0f34");
6472+
dataTO.setPath(diskTO.getPath());
6473+
dataTO.setDataStore(primaryDataStoreTO);
6474+
diskTO.setData(dataTO);
6475+
to.setDisks(new DiskTO[]{diskTO});
6476+
6477+
String path = "/dev/drbd1020";
6478+
final KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class);
6479+
final KVMStoragePool storagePool = Mockito.mock(KVMStoragePool.class);
6480+
final KVMPhysicalDisk vol = Mockito.mock(KVMPhysicalDisk.class);
6481+
6482+
when(libvirtComputingResourceSpy.getStoragePoolMgr()).thenReturn(storagePoolMgr);
6483+
when(storagePool.getType()).thenReturn(StoragePoolType.Linstor);
6484+
when(storagePoolMgr.getPhysicalDisk(StoragePoolType.Linstor, pDSTOUUID, diskLinPath)).thenReturn(vol);
6485+
when(vol.getPath()).thenReturn(path);
6486+
when(vol.getPool()).thenReturn(storagePool);
6487+
when(vol.getFormat()).thenReturn(PhysicalDiskFormat.RAW);
6488+
6489+
// 1. test Bus: IDE and broken qemu version -> NO discard
6490+
when(libvirtComputingResourceSpy.getHypervisorQemuVersion()).thenReturn(6000000L);
6491+
vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.IDE.name());
6492+
{
6493+
LibvirtVMDef vm = new LibvirtVMDef();
6494+
vm.addComp(new DevicesDef());
6495+
libvirtComputingResourceSpy.createVbd(connect, to, name, vm);
6496+
6497+
DiskDef rootDisk = vm.getDevices().getDisks().get(0);
6498+
assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType());
6499+
assertEquals(DiskDef.DiskBus.IDE, rootDisk.getBusType());
6500+
assertEquals(DiskDef.DiscardType.IGNORE, rootDisk.getDiscard());
6501+
}
6502+
6503+
// 2. test Bus: VIRTIO and broken qemu version -> discard unmap
6504+
vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.VIRTIO.name());
6505+
{
6506+
LibvirtVMDef vm = new LibvirtVMDef();
6507+
vm.addComp(new DevicesDef());
6508+
libvirtComputingResourceSpy.createVbd(connect, to, name, vm);
6509+
6510+
DiskDef rootDisk = vm.getDevices().getDisks().get(0);
6511+
assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType());
6512+
assertEquals(DiskDef.DiskBus.VIRTIO, rootDisk.getBusType());
6513+
assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard());
6514+
}
6515+
6516+
// 3. test Bus; IDE and "good" qemu version -> discard unmap
6517+
vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.IDE.name());
6518+
when(libvirtComputingResourceSpy.getHypervisorQemuVersion()).thenReturn(7000000L);
6519+
{
6520+
LibvirtVMDef vm = new LibvirtVMDef();
6521+
vm.addComp(new DevicesDef());
6522+
libvirtComputingResourceSpy.createVbd(connect, to, name, vm);
6523+
6524+
DiskDef rootDisk = vm.getDevices().getDisks().get(0);
6525+
assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType());
6526+
assertEquals(DiskDef.DiskBus.IDE, rootDisk.getBusType());
6527+
assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard());
6528+
}
6529+
6530+
// 4. test Bus: VIRTIO and "good" qemu version -> discard unmap
6531+
vmToDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, DiskDef.DiskBus.VIRTIO.name());
6532+
{
6533+
LibvirtVMDef vm = new LibvirtVMDef();
6534+
vm.addComp(new DevicesDef());
6535+
libvirtComputingResourceSpy.createVbd(connect, to, name, vm);
6536+
6537+
DiskDef rootDisk = vm.getDevices().getDisks().get(0);
6538+
assertEquals(DiskDef.DiskType.BLOCK, rootDisk.getDiskType());
6539+
assertEquals(DiskDef.DiskBus.VIRTIO, rootDisk.getBusType());
6540+
assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard());
6541+
}
6542+
}
6543+
64326544
@Test
64336545
public void recreateCheckpointsOnVmTestVersionIsNotSufficient() {
64346546
Mockito.doThrow(new CloudRuntimeException("")).when(libvirtComputingResourceSpy).validateLibvirtAndQemuVersionForIncrementalSnapshots();

‎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

‎plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java‎

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.cloud.hypervisor.kvm.storage;
1818

1919
import java.util.ArrayList;
20+
import java.util.Arrays;
2021
import java.util.Collections;
2122
import java.util.HashMap;
2223
import java.util.List;
@@ -48,6 +49,7 @@
4849
import com.linbit.linstor.api.model.Resource;
4950
import com.linbit.linstor.api.model.ResourceConnectionModify;
5051
import com.linbit.linstor.api.model.ResourceDefinition;
52+
import com.linbit.linstor.api.model.ResourceDefinitionModify;
5153
import com.linbit.linstor.api.model.ResourceGroupSpawn;
5254
import com.linbit.linstor.api.model.ResourceMakeAvailable;
5355
import com.linbit.linstor.api.model.ResourceWithVolumes;
@@ -235,6 +237,34 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu
235237
}
236238
}
237239

240+
private void setAllowTwoPrimariesOnRD(DevelopersApi api, String rscName) throws ApiException {
241+
ResourceDefinitionModify rdm = new ResourceDefinitionModify();
242+
Properties props = new Properties();
243+
props.put("DrbdOptions/Net/allow-two-primaries", "yes");
244+
props.put("DrbdOptions/Net/protocol", "C");
245+
rdm.setOverrideProps(props);
246+
ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm);
247+
if (answers.hasError()) {
248+
logger.error(String.format("Unable to set protocol C and 'allow-two-primaries' on %s", rscName));
249+
// do not fail here as adding allow-two-primaries property is only a problem while live migrating
250+
}
251+
}
252+
253+
private void setAllowTwoPrimariesOnRc(DevelopersApi api, String rscName, String inUseNode) throws ApiException {
254+
ResourceConnectionModify rcm = new ResourceConnectionModify();
255+
Properties props = new Properties();
256+
props.put("DrbdOptions/Net/allow-two-primaries", "yes");
257+
props.put("DrbdOptions/Net/protocol", "C");
258+
rcm.setOverrideProps(props);
259+
ApiCallRcList answers = api.resourceConnectionModify(rscName, inUseNode, localNodeName, rcm);
260+
if (answers.hasError()) {
261+
logger.error(String.format(
262+
"Unable to set protocol C and 'allow-two-primaries' on %s/%s/%s",
263+
inUseNode, localNodeName, rscName));
264+
// do not fail here as adding allow-two-primaries property is only a problem while live migrating
265+
}
266+
}
267+
238268
/**
239269
* Checks if the given resource is in use by drbd on any host and
240270
* if so set the drbd option allow-two-primaries
@@ -246,16 +276,13 @@ private void allow2PrimariesIfInUse(DevelopersApi api, String rscName) throws Ap
246276
String inUseNode = LinstorUtil.isResourceInUse(api, rscName);
247277
if (inUseNode != null && !inUseNode.equalsIgnoreCase(localNodeName)) {
248278
// allow 2 primaries for live migration, should be removed by disconnect on the other end
249-
ResourceConnectionModify rcm = new ResourceConnectionModify();
250-
Properties props = new Properties();
251-
props.put("DrbdOptions/Net/allow-two-primaries", "yes");
252-
props.put("DrbdOptions/Net/protocol", "C");
253-
rcm.setOverrideProps(props);
254-
ApiCallRcList answers = api.resourceConnectionModify(rscName, inUseNode, localNodeName, rcm);
255-
if (answers.hasError()) {
256-
logger.error("Unable to set protocol C and 'allow-two-primaries' on {}/{}/{}",
257-
inUseNode, localNodeName, rscName);
258-
// do not fail here as adding allow-two-primaries property is only a problem while live migrating
279+
280+
// if non hyperconverged setup, we have to set allow-two-primaries on the resource-definition
281+
// as there is no resource connection between diskless nodes.
282+
if (LinstorUtil.areResourcesDiskless(api, rscName, Arrays.asList(inUseNode, localNodeName))) {
283+
setAllowTwoPrimariesOnRD(api, rscName);
284+
} else {
285+
setAllowTwoPrimariesOnRc(api, rscName, inUseNode);
259286
}
260287
}
261288
}
@@ -294,11 +321,22 @@ public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<S
294321
return true;
295322
}
296323

297-
private void removeTwoPrimariesRcProps(DevelopersApi api, String inUseNode, String rscName) throws ApiException {
324+
private void removeTwoPrimariesRDProps(DevelopersApi api, String rscName, List<String> deleteProps)
325+
throws ApiException {
326+
ResourceDefinitionModify rdm = new ResourceDefinitionModify();
327+
rdm.deleteProps(deleteProps);
328+
ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm);
329+
if (answers.hasError()) {
330+
logger.error(
331+
String.format("Failed to remove 'protocol' and 'allow-two-primaries' on %s: %s",
332+
rscName, LinstorUtil.getBestErrorMessage(answers)));
333+
// do not fail here as removing allow-two-primaries property isn't fatal
334+
}
335+
}
336+
337+
private void removeTwoPrimariesRcProps(DevelopersApi api, String rscName, String inUseNode, List<String> deleteProps)
338+
throws ApiException {
298339
ResourceConnectionModify rcm = new ResourceConnectionModify();
299-
List<String> deleteProps = new ArrayList<>();
300-
deleteProps.add("DrbdOptions/Net/allow-two-primaries");
301-
deleteProps.add("DrbdOptions/Net/protocol");
302340
rcm.deleteProps(deleteProps);
303341
ApiCallRcList answers = api.resourceConnectionModify(rscName, localNodeName, inUseNode, rcm);
304342
if (answers.hasError()) {
@@ -310,6 +348,15 @@ private void removeTwoPrimariesRcProps(DevelopersApi api, String inUseNode, Stri
310348
}
311349
}
312350

351+
private void removeTwoPrimariesProps(DevelopersApi api, String inUseNode, String rscName) throws ApiException {
352+
List<String> deleteProps = new ArrayList<>();
353+
deleteProps.add("DrbdOptions/Net/allow-two-primaries");
354+
deleteProps.add("DrbdOptions/Net/protocol");
355+
356+
removeTwoPrimariesRDProps(api, rscName, deleteProps);
357+
removeTwoPrimariesRcProps(api, rscName, inUseNode, deleteProps);
358+
}
359+
313360
private boolean tryDisconnectLinstor(String volumePath, KVMStoragePool pool)
314361
{
315362
if (volumePath == null) {
@@ -343,7 +390,7 @@ private boolean tryDisconnectLinstor(String volumePath, KVMStoragePool pool)
343390
try {
344391
String inUseNode = LinstorUtil.isResourceInUse(api, rsc.getName());
345392
if (inUseNode != null && !inUseNode.equalsIgnoreCase(localNodeName)) {
346-
removeTwoPrimariesRcProps(api, inUseNode, rsc.getName());
393+
removeTwoPrimariesProps(api, inUseNode, rsc.getName());
347394
}
348395
} catch (ApiException apiEx) {
349396
logger.error(apiEx.getBestMessage());

0 commit comments

Comments
 (0)