-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
enabled discard option #10077
base: main
Are you sure you want to change the base?
enabled discard option #10077
Conversation
Enable the discard option for virtio-blk and virtio-scsi devices for volumes on StorPool storage
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10077 +/- ##
============================================
- Coverage 16.06% 16.06% -0.01%
- Complexity 12862 12863 +1
============================================
Files 5641 5642 +1
Lines 493791 493856 +65
Branches 59858 59860 +2
============================================
+ Hits 79318 79326 +8
- Misses 405691 405749 +58
+ Partials 8782 8781 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CLGTM
...rage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStoragePool.java
Outdated
Show resolved
Hide resolved
rearrange imports
Code looks ok, but we need to rely on your testing, @slavkap ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM.
minor: @slavkap would something like this improve readability?
--- a/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStoragePool.java
+++ b/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStoragePool.java
@@ -16,11 +16,13 @@
// under the License.
package com.cloud.hypervisor.kvm.storage;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
+import com.cloud.hypervisor.kvm.resource.LibvirtVMDef;
import com.cloud.storage.Storage;
import com.cloud.storage.Storage.StoragePoolType;
@@ -35,6 +37,11 @@ public class StorPoolStoragePool implements KVMStoragePool {
private String _sourceDir;
private String _localPath;
+ private static final List<LibvirtVMDef.DiskDef.DiskBus> supportedBusTypes = Arrays.asList(
+ LibvirtVMDef.DiskDef.DiskBus.VIRTIO,
+ LibvirtVMDef.DiskDef.DiskBus.SCSI
+ );
+
public StorPoolStoragePool(String uuid, String host, int port, StoragePoolType storagePoolType, StorageAdaptor storageAdaptor) {
_uuid = uuid;
_sourceHost = host;
@@ -166,4 +173,11 @@ public class StorPoolStoragePool implements KVMStoragePool {
public Map<String, String> getDetails() {
return null;
}
+
+ public void customizeLibvirtDiskDef(LibvirtVMDef.DiskDef disk) {
+ if (!supportedBusTypes.contains(disk.getBusType())) {
+ return;
+ }
+ disk.setDiscard(LibvirtVMDef.DiskDef.DiscardType.UNMAP);
+ }
}
Description
Enable the discard option for virtio-blk and virtio-scsi devices for volumes on StorPool storage
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
How Has This Been Tested?
manually tested with different controllers and UEFI legacy/secured
deploy a VM
attach volume