Skip to content

Extract memory validation into common function for processor modes#942

Open
spoorthys1303 wants to merge 1 commit into
open-power:masterfrom
spoorthys1303:machine_config
Open

Extract memory validation into common function for processor modes#942
spoorthys1303 wants to merge 1 commit into
open-power:masterfrom
spoorthys1303:machine_config

Conversation

@spoorthys1303

Copy link
Copy Markdown
Contributor

Changes:

  • Added validate_and_adjust_memory() method to LparConfig class
  • Validates and adjusts memory values to ensure min <= desired <= max
  • Maintains consistent validation behavior across both processor modes
  • Logs warnings when memory values are automatically adjusted

Changes:
- Added validate_and_adjust_memory() method to LparConfig class
- Validates and adjusts memory values to ensure min <= desired <= max
- Maintains consistent validation behavior across both processor modes
- Logs warnings when memory values are automatically adjusted

Signed-off-by: Spoorthy S <spoorts2@in.ibm.com>
@spoorthys1303

Copy link
Copy Markdown
Contributor Author

@PraveenPenguin fix for the memory allocation , Please review the PR

@abdhaleegit

Copy link
Copy Markdown
Collaborator

@spoorthys1303 is this the fix for the problem, where the memory layout for lpar is changed. after setting min and max ?

@spoorthys1303

Copy link
Copy Markdown
Contributor Author

@abdhaleegit issue in assigning memory was observed in ProfileSetup where max , desired memory is not mentioned in the config

@spoorthys1303

spoorthys1303 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

one situation where this script will set min_mem=4096,desired_mem=40960,max_mem=23552 .. min and default memory are default value coded in the script and max memory is calculated from available memory , and we get error that desired is greater than max and script will fail.. so this logic will make sure min<desired<max

@spoorthys1303

Copy link
Copy Markdown
Contributor Author

@PraveenPenguin should we handle how maximum memory is calculated from available memory and the steal able memory ?

@PraveenPenguin

Copy link
Copy Markdown
Collaborator

@PraveenPenguin should we handle how maximum memory is calculated from available memory and the steal able memory ?

Having maximum memory is calculated from available memory should be ok

@abdhaleegit

Copy link
Copy Markdown
Collaborator

@spoorthys1303 @PraveenPenguin if I have lets say max memory set for lpar as 30GB,, and when run with CR with this patch.. I want the max 30GB to be intact... and not worry about min and desired memory ? thats the CR concern I had.. all CR lpars were changed to max available memory.. will this patch fix this ?

@PraveenPenguin

Copy link
Copy Markdown
Collaborator

@spoorthys1303 @PraveenPenguin if I have lets say max memory set for lpar as 30GB,, and when run with CR with this patch.. I want the max 30GB to be intact... and not worry about min and desired memory ? thats the CR concern I had.. all CR lpars were changed to max available memory.. will this patch fix this ?

Good question 👍 — let me clarify how this behaves with the patch.

validate_and_adjust_memory() only enforces ordering (min ≤ desired ≤ max) and does not modify max_memory.
So if your LPAR is already configured with max = 30GB, that value will remain intact. The function will only adjust min/desired to fit within that bound. but we need to have fix as

So if your LPAR is already configured with max = 30GB, that value will remain intact. The function will only adjust min/desired to fit within that bound.

self.max_memory = int(self.cv_HMC.get_available_mem_resources()[0]) + \
                  self.cv_HMC.get_stealable_mem_resources_lpar()

So if max_memory is not explicitly set (AttributeError path), it gets reset to available system memory, which is exactly what caused CR LPARs earlier to move to full system capacity. that need to address separately not scope of this patch .. @spoorthys1303 see if you planning to fix this also; coordinate with @shirishaganta1 @SamirMulani if planning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants