Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

fix: Update overcommit ratio during live VM migration#9178

Merged
yadvr merged 2 commits intoapache:4.18apache/cloudstack:4.18from
shapeblue:fix-overcommit-vm-migrationshapeblue/cloudstack:fix-overcommit-vm-migrationCopy head branch name to clipboard
Jun 24, 2024
Merged

fix: Update overcommit ratio during live VM migration#9178
yadvr merged 2 commits intoapache:4.18apache/cloudstack:4.18from
shapeblue:fix-overcommit-vm-migrationshapeblue/cloudstack:fix-overcommit-vm-migrationCopy head branch name to clipboard

Conversation

@vishesh92
Copy link
Member

@vishesh92 vishesh92 commented Jun 5, 2024

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

  1. Set overcommit ratio to 2 for the cluster
  2. Launch a VM
  3. Check value of shares on the host
$ virsh dumpxml --domain i-2-9-VM | grep shares
    <shares>500</shares>
  1. Live migrate the VM to another host which has a different cgroup than the existing host of the VM.
  2. 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>
  1. 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

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

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
Copy link
Member Author

vishesh92 commented Jun 5, 2024

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Jun 5, 2024

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

@vishesh92 vishesh92 requested a review from sureshanaparti June 5, 2024 12:56
@vishesh92 vishesh92 changed the title Update overcommit ratio during live VM migration Update overcommit ratio during live VM migration across host with different cgroups Jun 5, 2024
@vishesh92 vishesh92 changed the title Update overcommit ratio during live VM migration across host with different cgroups fix: Update overcommit ratio during live VM migration Jun 5, 2024
@codecov
Copy link

codecov bot commented Jun 5, 2024 •
edited
Loading

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
Copy link

blueorangutan commented Jun 5, 2024

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

sureshanaparti
sureshanaparti approved these changes Jun 6, 2024
View reviewed changes
Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@sureshanaparti
Copy link
Contributor

sureshanaparti commented Jun 6, 2024

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Jun 6, 2024

@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
Copy link

blueorangutan commented Jun 6, 2024

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

@vishesh92
Copy link
Member Author

vishesh92 commented Jun 6, 2024

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Jun 6, 2024

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

@blueorangutan
Copy link

blueorangutan commented Jun 7, 2024

[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
yadvr approved these changes Jun 10, 2024
View reviewed changes
Copy link
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@vishesh92 vishesh92 added this to the 4.18.3 milestone Jun 12, 2024
@vishesh92
Copy link
Member Author

vishesh92 commented Jun 20, 2024

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Jun 20, 2024

@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
Copy link

blueorangutan commented Jun 20, 2024

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

@vishesh92
Copy link
Member Author

vishesh92 commented Jun 20, 2024

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Jun 20, 2024

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

@BryanMLima BryanMLima self-assigned this Jun 20, 2024
@blueorangutan
Copy link

blueorangutan commented Jun 21, 2024

[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
vladimirpetrov approved these changes Jun 21, 2024
View reviewed changes
Copy link
Contributor

@vladimirpetrov vladimirpetrov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@yadvr yadvr modified the milestones: 4.18.3, 4.19.1.0 Jun 24, 2024
@yadvr yadvr marked this pull request as ready for review June 24, 2024 15:13
@yadvr
Copy link
Member

yadvr commented Jun 24, 2024

Merging this based on reviews, smoketests and manual QA.

@yadvr yadvr merged commit 351de5f into apache:4.18 Jun 24, 2024
@yadvr yadvr deleted the fix-overcommit-vm-migration branch June 24, 2024 15:15
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jul 2, 2024
@vishesh92 @dhslove
engine/orchestration: Update overcommit ratio during live VM migration (
3ef18c7 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yadvr yadvr yadvr approved these changes

@sureshanaparti sureshanaparti sureshanaparti approved these changes

@vladimirpetrov vladimirpetrov vladimirpetrov approved these changes

Assignees

@vladimirpetrov vladimirpetrov

@BryanMLima BryanMLima

Labels

component:api component:compute component:orchestration

Projects

No open projects
Apache CloudStack 4.19.1
Status: No status

Milestone

4.19.1.0

Development

Successfully merging this pull request may close these issues.

7 participants

@vishesh92 @blueorangutan @sureshanaparti @yadvr @vladimirpetrov @DaanHoogland @BryanMLima
Morty Proxy This is a proxified and sanitized view of the page, visit original site.