Skip to content

Conversation

@adityagesh
Copy link
Collaborator

  1. Kdump currently has a lot of failures when sufficient disk space is not present, add disk using platform when additional space is required.
  2. Add fstab entry when partition used to store crashdump is not root partition - observed some cases where kdump service fails to start after rebooting due to mount issues.
  3. Refactoring

@adityagesh adityagesh requested a review from LiliDeng as a code owner December 9, 2025 15:31
Comment on lines +899 to +905
"""
from lisa.features import Disk

# Check if Disk feature is supported on this platform
if not self.node.features.is_supported(Disk):
raise LisaException("Disk feature is not supported on this platform")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@LiliDeng , I remember Chi mentioning tool should not import feature. But I do not see any other way to solve this. Do you have any suggestion?

Copy link
Contributor

Copilot AI left a 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 pull request enhances the kdump test suite to handle insufficient disk space scenarios and improve reliability across reboots. The changes address kdump failures when systems lack sufficient disk space by dynamically adding data disks when needed, and ensure mount points persist across reboots by adding fstab entries.

Key Changes:

  • Introduced a new Fstab tool for managing /etc/fstab entries programmatically
  • Enhanced kdump to calculate required disk space based on system memory and dynamically provision disks when needed
  • Added mount persistence logic to prevent kdump service failures after reboot
  • Refactored code to use tool abstractions (Mkdir, Ls) instead of raw execute commands
  • Fixed Oracle Linux grubby command to remove existing crashkernel parameters before adding new ones

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
lisa/tools/fstab.py New tool for managing /etc/fstab entries with methods to read, add, and ensure entries with UUID support
lisa/tools/kdump.py Enhanced disk space management with dynamic provisioning, added mount persistence, refactored to use tool abstractions, and improved Oracle Linux crashkernel configuration
lisa/tools/init.py Exported new Fstab and FstabEntry classes

Test Suggestions:

According to the LISA Testing Guidelines, all code reviews must include test suggestions. Here are the recommendations for this PR:

Key Test Cases:

verify_kdumpcrash_single_core|verify_kdumpcrash_smp|verify_kdumpcrash_auto_size|verify_kdumpcrash_large_memory_auto_size

Impacted LISA Features:
Disk, StartStop, SerialConsole

Tested Azure Marketplace Images:

  • canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
  • redhat rhel 9_5 latest
  • oracle oracle-linux ol94-lvm-gen2 latest
  • debian debian-12 12-gen2 latest
  • microsoftcblmariner azure-linux-3 azure-linux-3-gen2 latest

Rationale:

  • The test cases cover single-core, multi-core, auto-size crashkernel, and large memory scenarios which exercise the new disk provisioning and mount persistence logic
  • Oracle Linux images are critical to test the grubby command changes for crashkernel configuration
  • A variety of distributions ensures the fstab and disk management changes work across different OS families
  • Gen2 images are prioritized as they represent newer VM generations commonly used in production

adityagesh and others added 4 commits December 10, 2025 08:53
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
device_str = f"UUID={partition_info.uuid}"
self._log.debug(f"Using UUID for device {device}: {device_str}")
else:
self._log.warning(
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't use warning

f"Could not get UUID for {device}, using device name"
)
except LisaException as e:
self._log.warning(
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't use warning

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