[CLOUDSTACK-10352] XenServer: Support online migration of a virtual disk from non-managed to managed storage - #2502
Conversation
|
@mike-tutkowski a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
I plan to write a Marvin test for this, but - in the meanwhile - wanted to get this PR opened so reviewers could provide comments on the production-focused code. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1804 |
|
@mike-tutkowski can you check Travis failures, on this and your other recent PRs? |
|
@mike-tutkowski a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✖debian. JID-1813 |
|
@mike-tutkowski a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1824 |
|
@mike-tutkowski a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1844 |
bd3a4d0 to
389d080
Compare
borisstoyanov
left a comment
borisstoyanov
left a comment
There was a problem hiding this comment.
@mike-tutkowski since this is an enhancement do you think it's good to add a JIRA item and some documentation?
mike-tutkowski
changed the title
|
Hi @borisroman - I went ahead and created a JIRA ticket for this PR. I updated the title and description of this PR to include a reference to the ticket. I'm not sure if the CloudStack Community really keeps track of all of the permutations for online storage migration in terms of what currently is supported and not supported for a given release. In this situation, the difference the user would experience is that prior to 4.12 an error would be generated if he/she tried to perform this action while now, in 4.12, it works. |
|
Thanks @mike-tutkowski let me run the smoketests matrix. |
|
@borisstoyanov a Trillian-Jenkins matrix job (centos6 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2452)
|
|
Trillian test result (tid-2462)
|
borisstoyanov
left a comment
borisstoyanov
left a comment
There was a problem hiding this comment.
LGTM based on test results
|
@blueorangutan test matrix |
|
@PaulAngus a Trillian-Jenkins matrix job (centos6 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2487)
|
|
Trillian test result (tid-2486)
|
|
want to see some ci passing |
|
I have added a test suite for this feature. |
|
@syed Can you take a look at this PR? @borisroman has approved it. Also, Travis and Jenkins are green on it. This PR seems like something that may be of use to you guys at some point. Thanks! |
62ed673 to
586c76c
Compare
|
:) no Boris Roman has approved this, but welcome if you responded to the call back once a gain Schrijver. |
|
Whoops, I meant @borisstoyanov had approved it. :) |
586c76c to
f009c2d
Compare
|
I doubt this Jenkins error has anything to do this with PR: T E S T SRunning common.ClientTest Running streamer.ByteBufferTest Results : Tests in error: |
f009c2d to
90a3e1d
Compare
9e2493a to
23aec06
Compare
23aec06 to
aafbd19
Compare
aafbd19 to
e355506
Compare
e355506 to
772188b
Compare
772188b to
2e95e25
Compare
2e95e25 to
c8651f8
Compare
|
@syed Would you be able to take a look at this PR? CI looks good and the failed tests from the test matrix don't seem to have anything to do with this code. @borisroman has provided the first LGTM. Thanks! |
|
LGTM @mike-tutkowski So supposing I have a volume in a non-managed storage. I just change the disk offering and it would be migrated to managed storage via StorageMotion correct? |
|
@syed Actually, the way it works is that you would invoke the migrateVolume API command. For example, from a line in the tests: Volume.migrate(self.apiClient, livemigrate=True, volumeid=vm_root_volume.id, storageid=self.primary_storage_2.id) |
|
Here is a video that demonstrates this PR: https://www.youtube.com/watch?v=lkVMb6elvz4&t=12s&list=PLqOXKM0Bt13DFnQnwUx8ZtJzoyDV0Uuye&index=27 |
|
Here are the results from the new managed-storage test that is added in this PR: test_online_migrate_volume_from_nfs_storage_to_managed_storage (TestOnlineStorageMigration.TestOnlineStorageMigration) ... === TestName: test_online_migrate_volume_from_nfs_storage_to_managed_storage | Status : SUCCESS === Ran 2 tests in 523.545s OK |
|
Please note: To get this test to run properly, I had to comment out code in SnapshotManagerImpl.cleanupSnapshotByVolume. If that method is invoked, then any snapshot the volume was associated with prior to the migration of the volume from one primary storage to the next gets deleted. That is a serious problem and seems to have been introduced in #1740. |
80fdab2 to
24c917e
Compare
…ged storage to managed storage
5fdcc2c to
3db33b7
Compare
|
After updating and rebasing on top of master again, I was able to get the tests to pass without commenting out any snapshot-deletion code. test_online_migrate_volume_from_nfs_storage_to_managed_storage (TestOnlineStorageMigration.TestOnlineStorageMigration) ... === TestName: test_online_migrate_volume_from_nfs_storage_to_managed_storage | Status : SUCCESS === Ran 2 tests in 502.252s OK |
|
Two LGTMs and tests did well, so merging. |
7 participants
https://issues.apache.org/jira/browse/CLOUDSTACK-10352
Description
Allow on XenServer for a volume on non-managed storage to be online migrated to managed storage.
Types of changes
How Has This Been Tested?
Previously if you tried to online migrate a volume on XenServer from non-managed storage to managed storage, the operation failed. Now the operation succeeds.
Checklist:
@blueorangutan package