-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Migrate volume improvements, to bypass secondary storage when copy volume between pools is allowed directly #11625
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
base: main
Are you sure you want to change the base?
Conversation
@blueorangutan package |
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11625 +/- ##
=========================================
Coverage 17.39% 17.40%
- Complexity 15283 15296 +13
=========================================
Files 5889 5889
Lines 526183 526277 +94
Branches 64242 64262 +20
=========================================
+ Hits 91541 91582 +41
- Misses 424298 424337 +39
- Partials 10344 10358 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14980 |
@blueorangutan package |
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
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.
Pull Request Overview
This PR enhances the volume migration functionality to bypass secondary storage when direct copy between pools is possible, improving performance for supported pool types.
Key changes:
- Added
poolType
parameter to volume import/update operations to track storage pool types - Implemented logic to determine when secondary storage can be bypassed during volume migration
- Updated volume database records to include pool type information when pools change
Reviewed Changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
AncientDataMotionStrategy.java | Adds bypass logic for supported pool types (NFS, Filesystem) with scope validation |
VolumeOrchestrator.java | Updates import/update methods to include poolType parameter |
Multiple volume management files | Adds setPoolType() calls when updating volume pool assignments |
KVMStorageProcessor.java | Fixes path handling logic for volume copying between pools |
VMSnapshotManagerImpl.java | Corrects spelling error in exception messages |
Scope classes | Adds toString() methods for better debugging |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
Show resolved
Hide resolved
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 14981 |
@blueorangutan test |
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-14312)
|
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.
overall lgtm
not tested yet
...datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
Outdated
Show resolved
Hide resolved
...chestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
Show resolved
Hide resolved
d646d67
to
89813ff
Compare
@blueorangutan package |
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15031 |
@blueorangutan package |
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 15051 |
@blueorangutan package |
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15058 |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
1 similar comment
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
…lume between pools is allowed directly
- local storage on host in the same zone - cluser-wide pools in the same zone
… some code improvements
ad217ed
to
b134782
Compare
@blueorangutan package |
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15073 |
@blueorangutan test |
@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-14425)
|
Description
This PR improves migrate volume, to bypass secondary storage when copy volume between pools is allowed directly.
Also, updates the pool type of the volume wherever required.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Tested (offline) volume migration between:
How did you try to break this feature and the system with this change?