Linked clone migration between file-based storages on KVM by JoaoJandre · Pull Request #8911 · apache/cloudstack · GitHub

Description

Currently, when a volume that was created with linked clone is live migrated from an NFS storage to another NFS storage in KVM, it continues to have the template as a backing file on the destination storage. In all other cases, such as NFS to SharedMountPoint, the volume is consolidated with its backing file during migration.

This special case adds unnecessary complexity to ACS; In most cases, performing the migration without consolidating the volume does not significantly optimize resource usage. On the other hand, when consolidating the volume with its backing file, the hypervisor detects empty sectors and removes them from the final volume, therefore, consolidating the volume can reduce storage usage.

Instead of only maintaining linked clone only on NFS to NFS migration, we now maintain it when migrating between NFS, shared mount-point and local storage, as all of these are file based and have no reason to always consolidate the volumes.

In order to fix the situations explained in
#7615 and #8834, we consolidate the volume when its template is deleted.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

BLOCKER
Critical
Major
Minor
Trivial

Screenshots (if appropriate):

How Has This Been Tested?

I've tested creating a VM and migrating between NFS/SMP/Local storage, all migrations were successful, the template was copied to the storage and the VM was not corrupted.

I've also tested creating a VM, force deleting its template and then migrating it, the migration consolidated the volume, and the VM was not corrupted.

Codecov Report

Attention: Patch coverage is 0% with 35 lines in your changes missing coverage. Please review.

Project coverage is 15.28%. Comparing base (f0ba905) to head (f83c449).
Report is 65 commits behind head on 4.19.

Files with missing lines	Patch %	Lines
...torage/motion/StorageSystemDataMotionStrategy.java	0.00%	33 Missing ⚠️
...cloud/hypervisor/kvm/resource/MigrateKVMAsync.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.19    #8911      +/-   ##
============================================
+ Coverage     15.07%   15.28%   +0.20%     
- Complexity    11168    11580     +412     
============================================
  Files          5406     5406              
  Lines        472795   490142   +17347     
  Branches      57834    66027    +8193     
============================================
+ Hits          71282    74904    +3622     
- Misses       393585   406937   +13352     
- Partials       7928     8301     +373

Flag	Coverage Δ
uitests	5.10% <ø> (+0.79%)	⬆️
unittests	15.94% <0.00%> (+0.14%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@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]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9252

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9256

@blueorangutan package

@JoaoJandre 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9262

@sureshanaparti could we run de CI here?

@blueorangutan test

@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[SF] Trillian Build Failed (tid-9870)

[SF] Trillian Build Failed (tid-9873)

[SF] Trillian test result (tid-9884)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45095 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8911-t9884-kvm-centos7.zip
Smoke tests completed. 127 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_events_resource	Error	296.55	test_events_resource.py
test_01_events_resource	Error	296.56	test_events_resource.py
test_04_deploy_vm_for_other_user_and_test_vm_operations	Failure	85.55	test_network_permissions.py
ContextSuite context=TestNetworkPermissions>:teardown	Error	1.39	test_network_permissions.py

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@blueorangutan package

@DaanHoogland 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]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9536

@blueorangutan package

@JoaoJandre 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]: ✔️ el7 ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 9547

@blueorangutan package

@DaanHoogland 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 11026

@blueorangutan test

@rohityadavcloud a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

[SF] Trillian test result (tid-11411)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 50262 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8911-t11411-kvm-ol8.zip
Smoke tests completed. 132 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_unsecure_vm_migration	Error	446.79	test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration	Error	3954.86	test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration	Error	3955.01	test_vm_life_cycle.py
test_04_nonsecured_to_secured_vm_migration	Error	142.23	test_vm_life_cycle.py

tested by migrating a vm with storage to another host and its local storage,
Then stopped the VM and migrated the volume to shared storage and started the VM again

no issues

cc @sureshanaparti @DaanHoogland @JoaoJandre @weizhouapache is this considered critical for the release? I think this may be useful for local storage users for example, but not sure what others are considering.

@rohityadavcloud I would like to have it merged into 4.20. But we are still missing a review/LGTM. It probably will not make it.

LGTM! I tested this PR in a local environment by live migrating a VM with storage between NFS/Local/SMP as well as force-deleting a template and then migrating it, which worked perfectly fine as the volume was consolidated and the VM did not get corrupted.

LGTM based on manual testing on Oracle Linux 9 with NFS, local and shared mountpoint storages.

LGTM based on manual testing on Oracle Linux 9 with NFS, local and shared mountpoint storages.

great !

@JoaoJandre this has base branch 4.19. It is not really a fix, is it? Will we merge?

@JoaoJandre this has base branch 4.19. It is not really a fix, is it? Will we merge?

@DaanHoogland
it is a bug fix I think

In order to fix the situations explained in https://github.com/apache/cloudstack/pull/7615 and https://github.com/apache/cloudstack/pull/8834, we consolidate the volume when its template is deleted.

@JoaoJandre this has base branch 4.19. It is not really a fix, is it? Will we merge?

@DaanHoogland it is a bug fix I think

In order to fix the situations explained in https://github.com/apache/cloudstack/pull/7615 and https://github.com/apache/cloudstack/pull/8834, we consolidate the volume when its template is deleted.

ok, @weizhouapache just leaving it with the author/RM

@JoaoJandre this has base branch 4.19. It is not really a fix, is it? Will we merge?

@DaanHoogland it is a bug fix I think
In order to fix the situations explained in https://github.com/apache/cloudstack/pull/7615 and https://github.com/apache/cloudstack/pull/8834, we consolidate the volume when its template is deleted.

ok, @weizhouapache just leaving it with the author/RM

@DaanHoogland @weizhouapache I was originally not going to merge this as it was mainly created as a normalization PR. However, it is fixing a bug that has been reported in #7615 and #8834. This PR has also been extensively tested (see #8911 (comment), #8911 (review) and #8911 (review)) so we should be safe to merge it. Also, the failures in #8911 (comment) do not seem related:

2024-09-06 23:06:20,361 - CRITICAL - EXCEPTION: test_02_unsecure_vm_migration: ['Traceback (most recent call last):\n', '  File "/usr/lib64/python3.6/unittest/case.py", line 60, in testPartExecutor\n    yield\n', '  File "/usr/lib64/python3.6/unittest/case.py", line 622, in run\n    testMethod()\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1211, in test_02_unsecure_vm_migration\n    vm = self.deploy_vm(src_host)\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1154, in deploy_vm\n    hostid=origin_host.id)\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/lib/base.py", line 691, in create\n    virtual_machine = apiclient.deployVirtualMachine(cmd, method=method)\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/cloudstackAPI/cloudstackAPIClient.py", line 3981, in deployVirtualMachine\n    response = self.connection.marvinRequest(command, response_type=response, method=method)\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/cloudstackConnection.py", line 381, in marvinRequest\n    raise e\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/cloudstackConnection.py", line 376, in marvinRequest\n    raise self.__lastError\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/cloudstackConnection.py", line 105, in __poll\n    % async_response)\n', 'Exception: Job failed: {accountid : \'f8c22aaf-6c54-11ef-8280-1e009600027b\', account : \'admin\', domainid : \'ca87b0c6-6c54-11ef-8280-1e009600027b\', domainpath : \'ROOT\', userid : \'f8c3bd75-6c54-11ef-8280-1e009600027b\', cmd : \'org.apache.cloudstack.api.command.admin.vm.DeployVMCmdByAdmin\', jobstatus : 2, jobprocstatus : 0, jobresultcode : 530, jobresulttype : \'object\', jobresult : {errorcode : 530, errortext : \'Unable to start a VM [1826588c-f837-4e93-a5e8-696cbee3ab33] due to [Unable to create a deployment for VM instance {"id":366,"instanceName":"i-256-366-VM","type":"User","uuid":"1826588c-f837-4e93-a5e8-696cbee3ab33"}].\'}, jobinstancetype : \'VirtualMachine\', jobinstanceid : \'1826588c-f837-4e93-a5e8-696cbee3ab33\', created : \'2024-09-06T23:03:31+0000\', completed : \'2024-09-06T23:03:42+0000\', jobid : \'1d13a744-a414-43a6-b25e-384ff838c22f\'}\n']
2024-09-07 00:12:15,221 - CRITICAL - EXCEPTION: test_03_secured_to_nonsecured_vm_migration: ['Traceback (most recent call last):\n', '  File "/usr/lib64/python3.6/unittest/case.py", line 60, in testPartExecutor\n    yield\n', '  File "/usr/lib64/python3.6/unittest/case.py", line 622, in run\n    testMethod()\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1229, in test_03_secured_to_nonsecured_vm_migration\n    vm = self.deploy_vm(secure_host)\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1154, in deploy_vm\n    hostid=origin_host.id)\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/lib/base.py", line 691, in create\n    virtual_machine = apiclient.deployVirtualMachine(cmd, method=method)\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/cloudstackAPI/cloudstackAPIClient.py", line 3981, in deployVirtualMachine\n    response = self.connection.marvinRequest(command, response_type=response, method=method)\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/cloudstackConnection.py", line 381, in marvinRequest\n    raise e\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/cloudstackConnection.py", line 376, in marvinRequest\n    raise self.__lastError\n', '  File "/usr/local/lib/python3.6/site-packages/marvin/cloudstackConnection.py", line 105, in __poll\n    % async_response)\n', 'Exception: Job failed: {accountid : \'f8c22aaf-6c54-11ef-8280-1e009600027b\', account : \'admin\', domainid : \'ca87b0c6-6c54-11ef-8280-1e009600027b\', domainpath : \'ROOT\', userid : \'f8c3bd75-6c54-11ef-8280-1e009600027b\', cmd : \'org.apache.cloudstack.api.command.admin.vm.DeployVMCmdByAdmin\', jobstatus : 2, jobprocstatus : 0, jobresultcode : 530, jobresulttype : \'object\', jobresult : {errorcode : 530, errortext : \'Unable to start a VM [03dfcf15-bd8f-4ace-bdd1-115323402a2a] due to [Unable to create a deployment for VM instance {"id":367,"instanceName":"i-256-367-VM","type":"User","uuid":"03dfcf15-bd8f-4ace-bdd1-115323402a2a"}].\'}, jobinstancetype : \'VirtualMachine\', jobinstanceid : \'03dfcf15-bd8f-4ace-bdd1-115323402a2a\', created : \'2024-09-06T23:09:50+0000\', completed : \'2024-09-07T00:09:53+0000\', jobid : \'eb2b43c4-f75f-4efb-a4d0-741217fb3576\'}\n']
2024-09-07 00:12:15,373 - CRITICAL - EXCEPTION: test_03_secured_to_nonsecured_vm_migration: ['Traceback (most recent call last):\n', '  File "/usr/lib64/python3.6/unittest/case.py", line 60, in testPartExecutor\n    yield\n', '  File "/usr/lib64/python3.6/unittest/case.py", line 625, in run\n    self.tearDown()\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1067, in tearDown\n    self.secure_all_hosts()\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1144, in secure_all_hosts\n    self.check_connection(secured=\'true\', host=host)\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1174, in check_connection\n    ", actual is: " + host.details.secured)\n', "Exception: Host detail 'secured' was expected: true, actual is: false\n"]
2024-09-07 00:14:37,744 - CRITICAL - EXCEPTION: test_04_nonsecured_to_secured_vm_migration: ['Traceback (most recent call last):\n', '  File "/usr/lib64/python3.6/unittest/case.py", line 60, in testPartExecutor\n    yield\n', '  File "/usr/lib64/python3.6/unittest/case.py", line 618, in run\n    self.setUp()\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1063, in setUp\n    self.secure_all_hosts()\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1144, in secure_all_hosts\n    self.check_connection(secured=\'true\', host=host)\n', '  File "/marvin/tests/smoke/test_vm_life_cycle.py", line 1174, in check_connection\n    ", actual is: " + host.details.secured)\n', "Exception: Host detail 'secured' was expected: true, actual is: false\n"]

JoaoJandre added this to the 4.19.1.0 milestone Apr 12, 2024

boring-cyborg Bot added component:kvm component:storage labels Apr 12, 2024

sureshanaparti added the status:needs-testing label Apr 15, 2024

DaanHoogland mentioned this pull request Apr 18, 2024

Unstable 4.19 and main branch smoketests #8931

Closed

JoaoJandre mentioned this pull request Apr 19, 2024

KVM Incremental Snapshots/Backups #8907

Closed

gp-santos mentioned this pull request Apr 22, 2024

server: Fix NPE when migrating volume without template #7615

Closed

3 of 12 tasks

github-actions Bot added status:has-conflicts and removed status:has-conflicts labels Apr 29, 2024

JoaoJandre modified the milestones: 4.20.0.0, 4.21.0.0 Sep 10, 2024

JoaoJandre removed the status:needs-testing label Sep 12, 2024

FelipeM525 approved these changes Sep 30, 2024

View reviewed changes

vladimirpetrov approved these changes Oct 2, 2024

View reviewed changes

weizhouapache closed this Oct 2, 2024

weizhouapache reopened this Oct 2, 2024

JoaoJandre removed the status:needs-review label Oct 2, 2024

JoaoJandre merged commit e666dca into apache:4.19 Oct 2, 2024

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Oct 14, 2024

linked clone for file based storage (apache#8911 )

fbee39d

yadvr modified the milestones: 4.21.0, 4.20.0, 4.19.2 Nov 22, 2024

JoaoJandre mentioned this pull request Jan 2, 2025

Local storage root disk is melded with its template #9370

Closed

phsm mentioned this pull request Jan 28, 2025

VM migration with disks fails between local storage hypervisors if the VM template is deleted #10246

Closed

JoaoJandre mentioned this pull request May 15, 2025

KVM incremental snapshot feature #9270

Merged

3 of 13 tasks

Linked clone migration between file-based storages on KVM by JoaoJandre · Pull Request #8911 · apache/cloudstack · GitHub

Repository navigation

Linked clone migration between file-based storages on KVM - #8911

Linked clone migration between file-based storages on KVM#8911JoaoJandre merged 2 commits intoapache:4.19apache/cloudstack:4.19from scclouds:always-consolidate-volume-on-migrationscclouds/cloudstack:always-consolidate-volume-on-migrationCopy head branch name to clipboard

Conversation

JoaoJandre commented Apr 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

codecov-commenter commented Apr 12, 2024 • edited by codecov Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sureshanaparti commented Apr 15, 2024

Uh oh!

blueorangutan commented Apr 15, 2024

Uh oh!

blueorangutan commented Apr 15, 2024

Uh oh!

blueorangutan commented Apr 15, 2024

Uh oh!

JoaoJandre commented Apr 15, 2024

Uh oh!

blueorangutan commented Apr 15, 2024

Uh oh!

blueorangutan commented Apr 15, 2024

Uh oh!

JoaoJandre commented Apr 16, 2024

Uh oh!

DaanHoogland commented Apr 17, 2024

Uh oh!

blueorangutan commented Apr 17, 2024

Uh oh!

blueorangutan commented Apr 17, 2024

Uh oh!

blueorangutan commented Apr 17, 2024

Uh oh!

blueorangutan commented Apr 18, 2024

Uh oh!

github-actions Bot commented Apr 29, 2024

Uh oh!

DaanHoogland commented May 5, 2024

Uh oh!

blueorangutan commented May 5, 2024

Uh oh!

blueorangutan commented May 5, 2024

Uh oh!

JoaoJandre commented May 6, 2024

Uh oh!

blueorangutan commented May 6, 2024

Uh oh!

blueorangutan commented May 6, 2024

Uh oh!

DaanHoogland commented Sep 6, 2024

Uh oh!

blueorangutan commented Sep 6, 2024

Uh oh!

blueorangutan commented Sep 6, 2024

Uh oh!

yadvr commented Sep 6, 2024

Uh oh!

blueorangutan commented Sep 6, 2024

Uh oh!

blueorangutan commented Sep 7, 2024

Uh oh!

DaanHoogland commented Sep 9, 2024

Uh oh!

JoaoJandre commented Sep 9, 2024

Uh oh!

FelipeM525 left a comment

Choose a reason for hiding this comment

Uh oh!

vladimirpetrov left a comment

Choose a reason for hiding this comment

Linked clone migration between file-based storages on KVM#8911
JoaoJandre merged 2 commits into
apache:4.19from
scclouds:always-consolidate-volume-on-migration

JoaoJandre commented Apr 12, 2024 •
edited
Loading

codecov-commenter commented Apr 12, 2024 •
edited by codecov Bot
Loading