Copilot commented on code in PR #13468:
URL: https://github.com/apache/cloudstack/pull/13468#discussion_r3455029530
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -7022,7 +7022,7 @@ private static boolean isClvmVolume(DiskDef disk,
VirtualMachineTO vmSpec) {
continue;
}
VolumeObjectTO volumeTO = (VolumeObjectTO)
diskTO.getData();
- if (!diskPath.equals(volumeTO.getPath()) &&
!diskPath.equals(diskTO.getPath())) {
+ if
(!diskPath.substring(diskPath.lastIndexOf(File.separator) +
1).equals(volumeTO.getPath())) {
continue;
Review Comment:
This updated CLVM detection path-matching is subtle (e.g. `/dev/<vg>/<lv>`
vs `/dev/mapper/<vg>-<lv>` vs UUID-only paths). There are existing unit tests
for `LibvirtComputingResource`, but none cover `isClvmVolume`’s matching
behavior; adding a focused test would help prevent regressions in CLVM
migration handling.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -7022,7 +7022,7 @@ private static boolean isClvmVolume(DiskDef disk,
VirtualMachineTO vmSpec) {
continue;
}
VolumeObjectTO volumeTO = (VolumeObjectTO)
diskTO.getData();
- if (!diskPath.equals(volumeTO.getPath()) &&
!diskPath.equals(diskTO.getPath())) {
+ if
(!diskPath.substring(diskPath.lastIndexOf(File.separator) +
1).equals(volumeTO.getPath())) {
continue;
}
Review Comment:
The new disk-path matching compares the *basename* of `diskPath` against
`volumeTO.getPath()` (which is typically the full volume path, e.g.
`/dev/<vg>/<uuid>`). This will fail for common CLVM paths and can cause CLVM
volumes to be misclassified (skipping CLVM-specific handling during migration).
Normalize both paths (and keep the previous `diskTO.getPath()` fallback) by
comparing either full paths or basenames on both sides.
--
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]
