-
Notifications
You must be signed in to change notification settings - Fork 224
Hibernation: Add check for disk space requirement #4122
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
base: main
Are you sure you want to change the base?
Conversation
adityagesh
commented
Nov 21, 2025
- Hibernation requires free disk space proportional to RAM of the machine. LISA currently does not support os disk size as a requirement.
- Azure does not support Hibernation for VM with memory larger than 256 GB
1709020 to
ba9870a
Compare
|
LGTM |
6619dfd to
da8f248
Compare
| df_tool = node.tools[Df] | ||
|
|
||
| # Get total memory in KB | ||
| total_memory_kb = free_tool._get_field_bytes_kib("Mem", "total") |
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.
does get_total_memory meet your requirement?
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.
Thanks, updated
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
This PR adds disk space validation for VM hibernation to prevent setup failures. The changes implement pre-flight checks for disk space requirements and improve error handling for space-related hibernation failures.
- Adds a new function
check_hibernation_disk_requirements()that validates available disk space based on VM RAM size - Adds error pattern detection for defragmentation space errors in the hibernation setup tool
- Integrates the disk space check into the Power test suite's
before_casehook to fail fast before attempting hibernation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lisa/tools/hibernation_setup.py | Adds regex pattern and error handling for defragmentation space errors during hibernation setup |
| lisa/microsoft/testsuites/power/common.py | Implements new disk space validation function that checks RAM size and available disk space against hibernation requirements |
| lisa/microsoft/testsuites/power/power.py | Integrates disk space check into test suite setup by calling it in the before_case hook |
| # Get total memory in KB | ||
| total_memory_kb = free_tool._get_field_bytes_kib("Mem", "total") | ||
| total_memory_gb = total_memory_kb / (1024 * 1024) # Convert KB to GB |
Copilot
AI
Nov 24, 2025
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 accesses the private method _get_field_bytes_kib directly. Consider using the existing public method get_total_memory_gb() instead, which internally calls _get_field_bytes_kib("Mem", "total") >> 20. This would be more maintainable and follow the principle of using public APIs. If more precision is needed than the integer value returned by get_total_memory_gb(), consider adding a new public method to the Free class.
| # Get total memory in KB | |
| total_memory_kb = free_tool._get_field_bytes_kib("Mem", "total") | |
| total_memory_gb = total_memory_kb / (1024 * 1024) # Convert KB to GB | |
| # Get total memory in GB using public API | |
| total_memory_gb = float(free_tool.get_total_memory_gb()) |
| f"Current RAM: {total_memory_gb:.2f} GB" | ||
| ) | ||
|
|
||
| # Calculate required swap space based on RAM size |
Copilot
AI
Nov 24, 2025
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 comment states 'Calculate required swap space' but the code actually calculates required disk space for hibernation, not swap space. The comment should be updated to accurately reflect what's being calculated: 'Calculate required disk space based on RAM size'.
| # Calculate required swap space based on RAM size | |
| # Calculate required disk space for hibernation based on RAM size |
|
|
||
| # Get total memory in KB | ||
| total_memory_kb = free_tool._get_field_bytes_kib("Mem", "total") | ||
| total_memory_gb = total_memory_kb / (1024 * 1024) # Convert KB to GB |
Copilot
AI
Nov 24, 2025
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 conversion from KB to GB should use 1024 ** 2 or explicitly comment that this converts KiB to GiB (using binary units). The current comment 'Convert KB to GB' is technically imprecise since the values are in kibibytes (KiB) not kilobytes (KB), as indicated by the method name _get_field_bytes_kib.
| total_memory_gb = total_memory_kb / (1024 * 1024) # Convert KB to GB | |
| total_memory_gb = total_memory_kb / (1024 * 1024) # Convert KiB to GiB (binary units) |
| required_space_with_buffer = required_space_gb * 1.2 | ||
|
|
||
| root_partition = df_tool.get_partition_by_mountpoint("/", force_run=True) | ||
| assert root_partition is not None, "Unable to determine root partition disk space" |
Copilot
AI
Nov 24, 2025
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.
Using assert statements for error handling is not recommended in production code, as assertions can be disabled with Python's -O flag. Consider using an explicit if-check followed by raising a LisaException instead: if root_partition is None: raise LisaException(\"Unable to determine root partition disk space\").
| assert root_partition is not None, "Unable to determine root partition disk space" | |
| if root_partition is None: | |
| raise LisaException("Unable to determine root partition disk space") |
da8f248 to
7868802
Compare
1. Hibernation requires free disk space proportional to RAM of the machine. LISA currently does not support os disk size as a requirement. 2. Azure does not support Hibernation for VM with memory larger than 256 GB
7868802 to
99d6976
Compare