Skip to content

Commit 9423dcc

Browse files
weizhouapacheweizhouapache
committed
Merge remote-tracking branch 'apache/main' into 4.20-mgmt-server-peers
2 parents d20ac27 + 8af08dd commit 9423dcc

File tree

7 files changed

+178
-31
lines changed

7 files changed

+178
-31
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
@@ -381,6 +381,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
381381
protected static final String DEFAULT_TUNGSTEN_VIF_DRIVER_CLASS_NAME = "com.cloud.hypervisor.kvm.resource.VRouterVifDriver";
382382
private final static long HYPERVISOR_LIBVIRT_VERSION_SUPPORTS_IO_URING = 6003000;
383383
private final static long HYPERVISOR_QEMU_VERSION_SUPPORTS_IO_URING = 5000000;
384+
private final static long HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED = 7000000;
384385

385386
protected HypervisorType hypervisorType;
386387
protected String hypervisorURI;
@@ -3050,7 +3051,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
30503051
}
30513052
} else if (volume.getType() != Volume.Type.ISO) {
30523053
final PrimaryDataStoreTO store = (PrimaryDataStoreTO)data.getDataStore();
3053-
physicalDisk = storagePoolManager.getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath());
3054+
physicalDisk = getStoragePoolMgr().getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath());
30543055
pool = physicalDisk.getPool();
30553056
}
30563057

@@ -3152,7 +3153,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
31523153
else {
31533154
disk.defBlockBasedDisk(physicalDisk.getPath(), devId, diskBusType);
31543155
}
3155-
if (pool.getType() == StoragePoolType.Linstor) {
3156+
if (pool.getType() == StoragePoolType.Linstor && isQemuDiscardBugFree(diskBusType)) {
31563157
disk.setDiscard(DiscardType.UNMAP);
31573158
}
31583159
} else {
@@ -3299,6 +3300,16 @@ private boolean isIoUringEnabled() {
32993300
return isUbuntuHost() || isIoUringSupportedByQemu();
33003301
}
33013302

3303+
/**
3304+
* Qemu has a bug with discard enabled on IDE bus devices if qemu version < 7.0.
3305+
* <a href="https://bugzilla.redhat.com/show_bug.cgi?id=2029980">redhat bug entry</a>
3306+
* @param diskBus used for the disk
3307+
* @return true if it is safe to enable discard, otherwise false.
3308+
*/
3309+
public boolean isQemuDiscardBugFree(DiskDef.DiskBus diskBus) {
3310+
return diskBus != DiskDef.DiskBus.IDE || getHypervisorQemuVersion() >= HYPERVISOR_QEMU_VERSION_IDE_DISCARD_FIXED;
3311+
}
3312+
33023313
public boolean isUbuntuHost() {
33033314
Map<String, String> versionString = getVersionStrings();
33043315
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
@@ -1447,7 +1447,7 @@ protected synchronized void attachOrDetachDisk(final Connect conn, final boolean
14471447
diskdef.defFileBasedDisk(attachingDisk.getPath(), devId, busT, DiskDef.DiskFmtType.QCOW2);
14481448
} else if (attachingDisk.getFormat() == PhysicalDiskFormat.RAW) {
14491449
diskdef.defBlockBasedDisk(attachingDisk.getPath(), devId, busT);
1450-
if (attachingPool.getType() == StoragePoolType.Linstor) {
1450+
if (attachingPool.getType() == StoragePoolType.Linstor && resource.isQemuDiscardBugFree(busT)) {
14511451
diskdef.setDiscard(DiscardType.UNMAP);
14521452
}
14531453
}

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

‎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/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ public static void spLog(String fmt, Object... args) {
138138

139139
public static final String SP_TIER = "SP_QOSCLASS";
140140

141+
public static final String OBJECT_DOES_NOT_EXIST = "objectDoesNotExist";
142+
141143
public static enum StorpoolRights {
142144
RO("ro"), RW("rw"), DETACH("detach");
143145

@@ -458,7 +460,7 @@ public static JsonArray templatesStats(SpConnectionDesc conn) {
458460
}
459461

460462
private static boolean objectExists(SpApiError err) {
461-
if (!err.getName().equals("objectDoesNotExist")) {
463+
if (!err.getName().equals(OBJECT_DOES_NOT_EXIST)) {
462464
throw new CloudRuntimeException(err.getDescr());
463465
}
464466
return false;

‎plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java‎

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,19 @@
1616
// under the License.
1717
package org.apache.cloudstack.storage.snapshot;
1818

19-
import java.util.ArrayList;
20-
import java.util.List;
21-
22-
import javax.inject.Inject;
19+
import com.cloud.exception.InvalidParameterValueException;
20+
import com.cloud.hypervisor.kvm.storage.StorPoolStorageAdaptor;
21+
import com.cloud.storage.DataStoreRole;
22+
import com.cloud.storage.Snapshot;
23+
import com.cloud.storage.SnapshotVO;
24+
import com.cloud.storage.VolumeVO;
25+
import com.cloud.storage.dao.SnapshotDao;
26+
import com.cloud.storage.dao.SnapshotDetailsDao;
27+
import com.cloud.storage.dao.SnapshotDetailsVO;
28+
import com.cloud.storage.dao.SnapshotZoneDao;
29+
import com.cloud.storage.dao.VolumeDao;
30+
import com.cloud.utils.exception.CloudRuntimeException;
31+
import com.cloud.utils.fsm.NoTransitionException;
2332

2433
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
2534
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -40,23 +49,13 @@
4049
import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpApiResponse;
4150
import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpConnectionDesc;
4251
import org.apache.commons.collections.CollectionUtils;
43-
import org.apache.logging.log4j.Logger;
4452
import org.apache.logging.log4j.LogManager;
53+
import org.apache.logging.log4j.Logger;
4554
import org.springframework.stereotype.Component;
4655

47-
import com.cloud.exception.InvalidParameterValueException;
48-
import com.cloud.hypervisor.kvm.storage.StorPoolStorageAdaptor;
49-
import com.cloud.storage.DataStoreRole;
50-
import com.cloud.storage.Snapshot;
51-
import com.cloud.storage.SnapshotVO;
52-
import com.cloud.storage.VolumeVO;
53-
import com.cloud.storage.dao.SnapshotDao;
54-
import com.cloud.storage.dao.SnapshotDetailsDao;
55-
import com.cloud.storage.dao.SnapshotDetailsVO;
56-
import com.cloud.storage.dao.SnapshotZoneDao;
57-
import com.cloud.storage.dao.VolumeDao;
58-
import com.cloud.utils.exception.CloudRuntimeException;
59-
import com.cloud.utils.fsm.NoTransitionException;
56+
import javax.inject.Inject;
57+
import java.util.ArrayList;
58+
import java.util.List;
6059

6160

6261
@Component
@@ -117,10 +116,11 @@ public boolean deleteSnapshot(Long snapshotId, Long zoneId) {
117116
if (resp.getError() != null) {
118117
final String err = String.format("Failed to clean-up Storpool snapshot %s. Error: %s", name, resp.getError());
119118
StorPoolUtil.spLog(err);
120-
markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId, resp);
119+
markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId, resp.getError().getName().equals(StorPoolUtil.OBJECT_DOES_NOT_EXIST));
121120
throw new CloudRuntimeException(err);
122121
} else {
123122
res = deleteSnapshotFromDbIfNeeded(snapshotVO, zoneId);
123+
markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId,true);
124124
StorPoolUtil.spLog("StorpoolSnapshotStrategy.deleteSnapshot: executed successfully=%s, snapshot uuid=%s, name=%s", res, snapshotVO.getUuid(), name);
125125
}
126126
} catch (Exception e) {
@@ -129,15 +129,23 @@ public boolean deleteSnapshot(Long snapshotId, Long zoneId) {
129129
}
130130
}
131131

132+
List<SnapshotDataStoreVO> snapshots = _snapshotStoreDao.listBySnapshotIdAndState(snapshotId, State.Ready);
133+
if (res || CollectionUtils.isEmpty(snapshots)) {
134+
updateSnapshotToDestroyed(snapshotVO);
135+
return true;
136+
}
132137
return res;
133138
}
134139

135-
private void markSnapshotAsDestroyedIfAlreadyRemoved(Long snapshotId, SpApiResponse resp) {
136-
if (resp.getError().getName().equals("objectDoesNotExist")) {
137-
SnapshotDataStoreVO snapshotOnPrimary = _snapshotStoreDao.findBySourceSnapshot(snapshotId, DataStoreRole.Primary);
138-
if (snapshotOnPrimary != null) {
139-
snapshotOnPrimary.setState(State.Destroyed);
140-
_snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
140+
private void markSnapshotAsDestroyedIfAlreadyRemoved(Long snapshotId, boolean isSnapshotDeleted) {
141+
if (!isSnapshotDeleted) {
142+
return;
143+
}
144+
List<SnapshotDataStoreVO> snapshotsOnStore = _snapshotStoreDao.listBySnapshotIdAndState(snapshotId, State.Ready);
145+
for (SnapshotDataStoreVO snapshot : snapshotsOnStore) {
146+
if (snapshot.getInstallPath() != null && snapshot.getInstallPath().contains(StorPoolUtil.SP_DEV_PATH)) {
147+
snapshot.setState(State.Destroyed);
148+
_snapshotStoreDao.update(snapshot.getId(), snapshot);
141149
}
142150
}
143151
}

‎ui/src/views/infra/UpdatePrimaryStorage.vue‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ export default {
121121
},
122122
created () {
123123
this.initForm()
124-
this.form.name = this.resource.name
125124
},
126125
computed: {
127126
canUpdateNFSMountOpts () {
@@ -136,7 +135,14 @@ export default {
136135
methods: {
137136
initForm () {
138137
this.formRef = ref()
139-
this.form = reactive({ })
138+
this.form = reactive({
139+
name: this.resource.name,
140+
tags: this.resource.tags,
141+
isTagARule: this.resource.istagarule,
142+
capacityBytes: this.resource.disksizetotal,
143+
capacityIOPS: this.resource.capacityiops,
144+
nfsMountOpts: this.resource.nfsmountopts
145+
})
140146
this.rules = reactive({ })
141147
},
142148
isAdmin () {

0 commit comments

Comments
 (0)