Copilot commented on code in PR #14618:
URL: https://github.com/apache/cloudstack/pull/14618#discussion_r4507626000
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -3858,7 +3858,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
disk.setLogicalBlockIOSize(pool.getSupportedLogicalBlockSize());
disk.setPhysicalBlockIOSize(pool.getSupportedPhysicalBlockSize());
- if (diskBusType == DiskDef.DiskBus.SCSI || diskBusType ==
DiskDef.DiskBus.VIRTIOBLK) {
+ if (diskBusSupportsDiscard(diskBusType)) {
Review Comment:
This checks the root controller rather than the volume's final bus: with
`rootDiskController=virtio` and `dataDiskController=ide`, an NFS data disk now
receives UNMAP before it is defined as IDE. That bypasses the intended IDE
exclusion and exposes the QEMU < 7.0 IDE-discard bug documented by
`isQemuDiscardBugFree()`. Apply the predicate to `disk.getBusType()` after
storage-specific disk definition, preserve the version-gated Linstor exception,
and add a mixed-controller regression test.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -4266,7 +4270,7 @@ public synchronized String attachOrDetachDisk(final
Connect conn,
}
diskdef = new DiskDef();
- if (busT == DiskDef.DiskBus.SCSI) {
+ if (diskBusSupportsDiscard(busT)) {
Review Comment:
Live volume attachment goes through `KVMStorageProcessor.attachVolume()`,
not this method, and its `attachOrDetachDisk()` still enables discard only for
SCSI/VIRTIOBLK (`KVMStorageProcessor.java:1677`). Hot-attached NFS virtio
volumes therefore still ignore discard. Make the shared predicate accessible to
that implementation, apply it there, and add an attach-path regression test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
