-
Notifications
You must be signed in to change notification settings - Fork 394
T7887: system_option: Add validation for memory hugepage-count #4772
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
Conversation
|
👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds validation to ensure configured hugepage memory does not exceed available system memory and refactors duplicated hugepage size aggregation logic into a helper. Updates smoketest configurations to align with the new validation logic.
- Introduces _get_total_hugepages_and_memory helper to remove duplicated logic.
- Adds hugepage memory availability validation in verify().
- Adjusts test configs to lower hugepage-count values.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/conf_mode/system_option.py | Adds hugepage aggregation helper and validation logic; refactors apply() to reuse helper. |
| smoketest/configs/vpp | Updates hugepage-count to fit new validation constraints. |
| smoketest/config-tests/vpp | Updates hugepage-count in test setup to align with validation. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
e0c1e14 to
c128912
Compare
Move smoke test for hugepages to test_vpp.py
|
CI integration 👍 passed! Details
|
zdc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks fine to me.
I have not tested it.
dmbaturin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks sensible, so I think it's a good time to start testing it.
Change summary
Types of changes
Related Task(s)
Related PR(s)
How to test / Smoketest result
Checklist: