CLVM: Fix volume mapping and disk path matching for storage migration#13468
CLVM: Fix volume mapping and disk path matching for storage migration#13468Pearl1594 wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13468 +/- ##
=========================================
Coverage 18.89% 18.89%
- Complexity 18223 18228 +5
=========================================
Files 6174 6174
Lines 555226 555244 +18
Branches 67774 67778 +4
=========================================
+ Hits 104885 104896 +11
- Misses 438820 438826 +6
- Partials 11521 11522 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves CLVM behavior around storage migration/attach by refining when volumes are mapped for migration, updating CLVM lock-host tracking after volume moves, and simplifying endpoint selection during snapshot revert.
Changes:
- Simplifies
revertSnapshotendpoint selection by removing a CLVM-specific branch and consistently selecting based onsnapshotOnPrimaryStorevs base volume. - Improves CLVM lock-host handling: detect when cross-pool lock transfer is required and persist lock-host updates after volume copy/migration.
- Expands KVM migration volume-to-pool mapping to include CLVM pools and adjusts CLVM disk-path matching logic.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java | Removes CLVM-only endpoint selection during snapshot revert, selecting endpoint based on snapshot-on-primary presence. |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java | Updates CLVM disk-path matching logic used to detect CLVM volumes during migration handling. |
| engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java | Adds cross-pool lock-transfer detection based on tracked/derived CLVM lock host vs VM host. |
| engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java | Persists CLVM lock-host ID after successful volume copy/migrate operations to reduce fan-out/extra discovery. |
| engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java | Ensures volumes on CLVM pools are included in volume-to-pool mapping for KVM migrations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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; | ||
| } |
|
@blueorangutan package |
|
@Pearl1594 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 23.51% |
| Branch coverage | 17.70% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18335 |
|
@blueorangutan test |
|
@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Description
This PR addresses:
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?