Skip to content
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

Memory Tuning GSoC #57636

Merged
merged 3 commits into from Sep 25, 2020
Merged

Memory Tuning GSoC #57636

merged 3 commits into from Sep 25, 2020

Conversation

gqlo
Copy link
Contributor

@gqlo gqlo commented Jun 11, 2020

What does this PR do?

Feature request closes #57639

This PR makes memory tuning option available which allow much greater control of memory allocation. configurable parameters are:

  • hard_limit
  • soft_limit
  • swap_hard_limit
  • min_guarantee

In addition, _handle_unit function is added which uses regex to parse user input into the corresponding size.

Fixes a bug on adding sub element to existing XML tree. Element.append() should be used to append an element into existing XML tree. When using ElementTree.SubElement() we must make sure the parent element is from the target domain XML tree.

Refactored XML diff code, added a few helper functions to manipulate xml node.

More robust check is added if 'initrd' in boot and boot.initrd to ensure None object will not cause an exception if it is passed to initialise a new VM .

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes/No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@gqlo gqlo requested a review from a team as a code owner June 11, 2020 08:33
@ghost ghost requested review from dwoz and removed request for a team June 11, 2020 08:33
cbosdo
cbosdo previously approved these changes Jun 11, 2020
Copy link
Contributor

@cbosdo cbosdo left a comment

Choose a reason for hiding this comment

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

A few cosmetic comments, but nothing serious. ACK

salt/templates/virt/libvirt_domain.jinja Outdated Show resolved Hide resolved
tests/unit/modules/test_virt.py Outdated Show resolved Hide resolved
@gqlo gqlo changed the title Memory tune Memory Tuning Support Jun 11, 2020
@gqlo gqlo requested a review from cbosdo June 11, 2020 16:02
@sagetherage sagetherage added the Feature new functionality including changes to functionality and code refactors, etc. label Jun 11, 2020
cbosdo
cbosdo previously approved these changes Jun 12, 2020
Copy link
Contributor

@cbosdo cbosdo left a comment

Choose a reason for hiding this comment

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

👏

@gqlo gqlo force-pushed the memory_tune branch 10 times, most recently from 5fa3e80 to 7f066cf Compare July 19, 2020 12:51
@sagetherage sagetherage added the Magnesium Mg release after Na prior to Al label Jul 22, 2020
@gqlo gqlo marked this pull request as draft July 29, 2020 02:15
@gqlo gqlo force-pushed the memory_tune branch 6 times, most recently from bd6118d to 9018312 Compare July 29, 2020 12:35
Copy link
Contributor

@cbosdo cbosdo left a comment

Choose a reason for hiding this comment

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

3 small things to improve still and we're good!

salt/modules/virt.py Outdated Show resolved Hide resolved
salt/utils/xmlutil.py Show resolved Hide resolved
tests/unit/modules/test_virt.py Outdated Show resolved Hide resolved
@gqlo gqlo force-pushed the memory_tune branch 3 times, most recently from 7a16ea4 to 924bebe Compare September 8, 2020 08:33
cbosdo
cbosdo previously approved these changes Sep 8, 2020
Copy link
Contributor

@cbosdo cbosdo left a comment

Choose a reason for hiding this comment

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

👍 ready from my POV

@cbosdo
Copy link
Contributor

cbosdo commented Sep 8, 2020

@Akm0d could you review that PR? it should be good to go IMO

@cbosdo
Copy link
Contributor

cbosdo commented Sep 9, 2020

@dwoz could you merge this PR while it's ACKed, tests are green and doesn't need rebase yet?

@gqlo
Copy link
Contributor Author

gqlo commented Sep 11, 2020

re-run all

Akm0d
Akm0d previously approved these changes Sep 14, 2020
@cbosdo
Copy link
Contributor

cbosdo commented Sep 16, 2020

@dwoz would be nice to have this PR merged before the freeze

Akm0d
Akm0d previously approved these changes Sep 18, 2020
@cbosdo
Copy link
Contributor

cbosdo commented Sep 22, 2020

re-run lint

@cbosdo
Copy link
Contributor

cbosdo commented Sep 25, 2020

@Akm0d can you approve that PR again? had to be rebased due to pre-commit hook changes.

@dwoz dwoz merged commit 3760ba1 into saltstack:master Sep 25, 2020
@s0undt3ch
Copy link
Member

This introduced a test failure in our test suite

https://jenkinsci.saltstack.com/view/Master%20Branch%20Fast%20Jobs/job/pr-centos7-py3/job/master/1477/

@gqlo can you take a look at it please?

/cc @cbosdo

@gqlo
Copy link
Contributor Author

gqlo commented Sep 26, 2020

This introduced a test failure in our test suite

https://jenkinsci.saltstack.com/view/Master%20Branch%20Fast%20Jobs/job/pr-centos7-py3/job/master/1477/

@gqlo can you take a look at it please?

/cc @cbosdo

Hi Pedro, Just fixed the issue in this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. Magnesium Mg release after Na prior to Al Salt-Cloud
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory Backing Support Memory Tuning Support
6 participants