fix: Update overcommit ratio during live VM migration by vishesh92 · Pull Request #9178 · apache/cloudstack

vishesh92 · 2024-06-05T12:34:52Z

Description

During live migration of a VM from between hosts having different cgroup versions (cgroupv2 & cgroup), overcommit ratio is ignored.

This PR fixes the above issue.

Steps to reproduce

Set overcommit ratio to 2 for the cluster
Launch a VM
Check value of shares on the host

$ virsh dumpxml --domain i-2-9-VM | grep shares
<shares>500</shares>

Live migrate the VM to another host which has a different cgroup than the existing host of the VM.
Live migrate the VM back to the original host. And check the shares again.

$ virsh dumpxml --domain i-2-9-VM | grep shares
<shares>1000</shares>

Stopping and starting the VM on the same host will reset the value back to 500.
This happens only when cgroup is different on the destination hosts during live migration. Possibly introduced in kvm: Add support for cgroupv2 #8252 .

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

Screenshots (if appropriate):

How Has This Been Tested?

Not able to reproduce the issue after the patch.

How did you try to break this feature and the system with this change?

vishesh92 · 2024-06-05T12:35:47Z

@blueorangutan package

blueorangutan · 2024-06-05T12:38:03Z

@vishesh92 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 · 2024-06-05T13:27:17Z

Codecov Report

Attention: Patch coverage is 60.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 12.24%. Comparing base (c791c13) to head (9ec7b17).
Report is 2 commits behind head on 4.18.

Files	Patch %	Lines
...n/java/com/cloud/vm/VirtualMachineManagerImpl.java	60.00%	4 Missing and 4 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## 4.18 #9178 +/- ##
============================================
- Coverage 12.24% 12.24% -0.01% 
Complexity 9291 9291 ============================================
Files 4698 4698 Lines 414259 414266 +7 Branches 51107 53086 +1979 ============================================
- Hits 50709 50707 -2 - Misses 357248 357258 +10 + Partials 6302 6301 -1

Flag	Coverage Δ
unittests	`12.24% <60.00%> (-0.01%)`	⬇️

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 · 2024-06-05T13:38:08Z

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

sureshanaparti

clgtm

sureshanaparti · 2024-06-06T09:14:45Z

@blueorangutan package

blueorangutan · 2024-06-06T09:16:04Z

@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.

blueorangutan · 2024-06-06T10:25:39Z

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

vishesh92 · 2024-06-06T20:43:32Z

@blueorangutan test

blueorangutan · 2024-06-06T20:44:04Z

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

blueorangutan · 2024-06-07T09:34:03Z

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

Test	Result	Time (s)	Test File
ContextSuite context=TestAnnotations>:setup	`Error`	0.00	test_annotations.py
test_query_async_job_result	`Error`	0.00	test_async_job.py
ContextSuite context=TestNetworkACL>:setup	`Error`	0.00	test_network_acl.py
test_nic_secondaryip_add_remove	`Failure`	0.07	test_multipleips_per_nic.py
ContextSuite context=TestNestedVirtualization>:setup	`Error`	0.00	test_nested_virtualization.py
test_01_verify_ipv6_network	`Failure`	1.66	test_network_ipv6.py
test_08_migrate_vm	`Error`	43.85	test_vm_life_cycle.py
test_02_cancel_host_maintenace_with_migration_jobs	`Error`	199.94	test_host_maintenance.py
test_hostha_kvm_host_fencing	`Error`	109.36	test_hostha_kvm.py

yadvr

LGTM - didn't test it but wondering if it can have any side-effects?

vishesh92 · 2024-06-20T15:40:39Z

@blueorangutan package

blueorangutan · 2024-06-20T15:42:04Z

@vishesh92 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.

blueorangutan · 2024-06-20T16:53:44Z

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

vishesh92 · 2024-06-20T16:56:24Z

@blueorangutan test

blueorangutan · 2024-06-20T16:58:03Z

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

blueorangutan · 2024-06-21T06:21:06Z

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

Test	Result	Time (s)	Test File
test_02_cancel_host_maintenace_with_migration_jobs	`Error`	191.96	test_host_maintenance.py
test_hostha_kvm_host_fencing	`Error`	185.02	test_hostha_kvm.py

vladimirpetrov

LGTM based on manual testing, tested on Oracle Linux 9.

yadvr · 2024-06-24T15:14:55Z

Merging this based on reviews, smoketests and manual QA.

apache#9178) During live migration of a VM from between hosts having different cgroup versions (cgroupv2 & cgroup), overcommit ratio is ignored. This PR fixes the above issue.

Update overcommit ratio during live VM migration
6cf3a06

boring-cyborgBot added component:api component:compute component:orchestration labels Jun 5, 2024

vishesh92 requested a review from sureshanaparti June 5, 2024 12:56

vishesh92 changed the title ~~Update overcommit ratio during live VM migration~~Update overcommit ratio during live VM migration across host with different cgroupsJun 5, 2024

vishesh92 changed the title ~~Update overcommit ratio during live VM migration across host with different cgroups~~fix: Update overcommit ratio during live VM migrationJun 5, 2024

sureshanaparti reviewed Jun 6, 2024
View reviewed changes

Comment threadengine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java Outdated

sureshanaparti reviewed Jun 6, 2024
View reviewed changes

Comment threadengine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

minor refactoring
9ec7b17

sureshanaparti approved these changes Jun 6, 2024
View reviewed changes

yadvr approved these changes Jun 10, 2024
View reviewed changes

DaanHoogland added the status:needs-testing label Jun 11, 2024

vishesh92 added this to the 4.18.3 milestone Jun 12, 2024

yadvr assigned vladimirpetrov Jun 18, 2024

BryanMLima self-assigned this Jun 20, 2024

vladimirpetrov approved these changes Jun 21, 2024
View reviewed changes

yadvr modified the milestones: 4.18.3, 4.19.1.0 Jun 24, 2024

yadvr marked this pull request as ready for review June 24, 2024 15:13

yadvr removed the status:needs-testing label Jun 24, 2024

yadvr merged commit 351de5f into apache:4.18Jun 24, 2024

yadvr deleted the fix-overcommit-vm-migration branch June 24, 2024 15:15

Uh oh!

Conversation

vishesh92 commented Jun 5, 2024

Description

Steps to reproduce

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?

Uh oh!

vishesh92 commented Jun 5, 2024

Uh oh!

blueorangutan commented Jun 5, 2024

Uh oh!

codecovBot commented Jun 5, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented Jun 5, 2024

Uh oh!

Uh oh!

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Jun 6, 2024

Uh oh!

blueorangutan commented Jun 6, 2024

Uh oh!

blueorangutan commented Jun 6, 2024

Uh oh!

vishesh92 commented Jun 6, 2024

Uh oh!

blueorangutan commented Jun 6, 2024

Uh oh!

blueorangutan commented Jun 7, 2024

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

vishesh92 commented Jun 20, 2024

Uh oh!

blueorangutan commented Jun 20, 2024

Uh oh!

blueorangutan commented Jun 20, 2024

Uh oh!

vishesh92 commented Jun 20, 2024

Uh oh!

blueorangutan commented Jun 20, 2024

Uh oh!

blueorangutan commented Jun 21, 2024

Uh oh!

vladimirpetrov left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Jun 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecovBot commented Jun 5, 2024•
edited
Loading