io/disk: Added disk_cleanup module and refactor test teardown [depended on util PR : 6312] - #3188
Conversation
7d5544d to
5f629df
Compare
|
RHEL run TestSuite TestRun Summary host_io_disk_cleanup_bonnie_bonnie Run Successfully executed host_io_disk_cleanup_tiobench_tiobench Run Successfully executed host_io_disk_cleanup_disk_info_disk_info Run Successfully executed host_io_disk_cleanup_disktest_disktest Run Successfully executed host_io_disk_cleanup_fiotest_fio Run Successfully executed host_io_disk_cleanup_softwareraid_softwareraid Run Successfully executed host_io_disk_cleanup_ltp_fsstress_ltp_fsstress Run Successfully executed host_io_disk_cleanup_ltp_fs_ltp_fs Run Successfully executed host_io_disk_cleanup_parallel_dd_parallel_dd Run Successfully executed host_io_disk_cleanup_fs_mark_fs_mark Run Successfully executed host_io_disk_cleanup_ioping_ioping Run Successfully executed host_io_disk_cleanup_iozone_iozone Run Successfully executed host_io_disk_cleanup_lvsetup_lvsetup Run Successfully executed SUSE bucket run host_io_disk_cleanup_bonnie_bonnie Run Successfully executed host_io_disk_cleanup_tiobench_tiobench Run Successfully executed host_io_disk_cleanup_disk_info_disk_info Run Successfully executed host_io_disk_cleanup_disktest_disktest Run Successfully executed host_io_disk_cleanup_fiotest_fio Run Successfully executed host_io_disk_cleanup_softwareraid_softwareraid Run Successfully executed host_io_disk_cleanup_ltp_fsstress_ltp_fsstress Run Successfully executed host_io_disk_cleanup_ltp_fs_ltp_fs Run Successfully executed host_io_disk_cleanup_parallel_dd_parallel_dd Run Successfully executed host_io_disk_cleanup_fs_mark_fs_mark Run Successfully executed host_io_disk_cleanup_ioping_ioping Run Successfully executed host_io_disk_cleanup_iozone_iozone Run Successfully executed host_io_disk_cleanup_lvsetup_lvsetup Run Successfully executed No issues observed with the the flow with respect to io_bucket/CR flow. |
35e614e to
7aa7868
Compare
|
@Naresh-ibm @abdhaleegit @PraveenPenguin @sacsant The current cleanup code PR is an advancement to the existing flow. Advantages:
Let me know if required any other information |
7aa7868 to
be707c0
Compare
PraveenPenguin
left a comment
There was a problem hiding this comment.
I feel move cleanup disk utility on avocado it self
be707c0 to
e80c150
Compare
|
Moved all the disk cleanup utilities to avocado framework Update all the files to use utils from avocado CR run is also successful without any issues TestSuite TestRun Summary host_io_disk_cleanup_bonnie_bonnie Run Successfully executed host_io_disk_cleanup_rawread_rawread Run Successfully executed host_io_disk_cleanup_tiobench_tiobench Run Successfully executed host_io_disk_cleanup_disktest_disktest Run Successfully executed host_io_disk_cleanup_fiotest_fio Run Successfully executed host_io_disk_cleanup_softwareraid_softwareraid Run Successfully executed host_io_disk_cleanup_ltp_fsstress_ltp_fsstress Run Successfully executed host_io_disk_cleanup_ltp_fs_ltp_fs Run Successfully executed host_io_disk_cleanup_parallel_dd_parallel_dd Run Successfully executed host_io_disk_cleanup_fs_mark_fs_mark Run Successfully executed host_io_disk_cleanup_ioping_ioping Run Successfully executed host_io_disk_cleanup_iozone_iozone Run Successfully executed host_io_disk_cleanup_lvsetup_lvsetup Run Successfully executed |
|
Just for the Information As this PR depends on utils PR avocado-framework/avocado#6312 It should be merged only after successful merge on utils PR |
This commit refactors disk I/O test teardown to use centralized cleanup utilities from the avocado framework's disk utility module instead of local implementations. Key changes: - Remove local disk_cleanup.py module (functionality migrated to avocado.utils.disk) - Update 13 test files to import cleanup_disks from avocado.utils.disk: * bonnie.py, dbench.py, disk_info.py, disktest.py, fiotest.py * fs_mark.py, iozone.py, ltp_fs.py, ltp_fsstress.py, lvsetup.py * parallel_dd.py, softwareraid.py, tiobench.py - Improve RAID cleanup: Use log.info for forced stop (normal flow) instead of log.warning to reduce noise in test logs Benefits: - Centralized maintenance in avocado framework - Consistent cleanup behavior across all disk tests - Disk-type agnostic utilities (NVMe, SCSI, IDE, virtio, etc.) - Dependency-aware cleanup order (mounts → swap → LVM → RAID → metadata) - Enhanced error handling and logging The cleanup utilities in avocado.utils.disk provide: - Automatic multipath device detection and normalization - Intelligent dependency graph resolution - Auto-detection mode for 'light' or 'full' cleanup - Orphaned device-mapper and VG directory cleanup - Enhanced RAID array detection using /sys/block slaves - Comprehensive metadata wiping with retry logic Signed-off-by: Maram Srimannarayana Murthy <msmurthy@linux.vnet.ibm.com>
e80c150 to
aad979a
Compare
|
|
||
| dmesg.clear_dmesg() | ||
| self.pre_cleanup() | ||
| cleanup_disks([self.disk], logger=self.log) |
There was a problem hiding this comment.
With this change pre_cleanup() function is now a dead code. It is not called from any other function.
setUp() should call pre_cleanup(). Can you fix this?
There was a problem hiding this comment.
Compared to other py files, cleanup_disks() is directly called without any error handling. All other tests use a try block around cleanup_disks().
| """ | ||
| Cleanup of disk used to perform this test | ||
| """ | ||
| if self.fs_create: |
There was a problem hiding this comment.
In tearDown() there is a call to delete_lv()
The tearDown() first calls self.delete_lv() (which removes the LV/VG), and then also calls cleanup_disks(self.disks, mode="full") which would attempt to remove LV/VG/RAID again. While this is likely harmless, it creates redundancy and potential confusing log messages.
More importantly, if self.delete_lv() fails with an exception, cleanup_disks() will never be reached.
Can you check? Either move delete_lv() after cleanup_disks() or add a try block to handle exception and continue
| from avocado.utils.partition import Partition | ||
| from avocado.utils.software_manager.manager import SoftwareManager | ||
| from avocado.utils.partition import PartitionError | ||
| from avocado.utils.disk import cleanup_disks |
There was a problem hiding this comment.
There is already a import statement
from avocado.utils import disk
Do we need this additional import?
from avocado.utils.disk import cleanup_disks
Applies to all tests
| self.log.info("Pre-cleanup completed successfully") | ||
| except Exception as e: | ||
| self.cancel("Pre-cleanup failed, cannot run on dirty disk: %s" % e) | ||
| self.log.info("Running dd...") |
There was a problem hiding this comment.
This self.log.info() is not required
| if hasattr(self, "sraid"): | ||
| self.sraid.stop() | ||
| self.sraid.clear_superblock() | ||
|
|
There was a problem hiding this comment.
The tearDown stops the RAID and clears superblock manually, then calls cleanup_disks() in "full" mode which would also attempt RAID stop.
Since cleanup_disks handles RAID cleanup, the manual stop/clear_superblock is redundant (though harmless). If the intent is to fully rely on cleanup_disks(), the manual calls should be removed.
There was a problem hiding this comment.
@maramsmurthy One concern, hope this works for multipath san luns, vscsi, nvme, single path scsi .. etc
I see the logs explicitly runs only cleanup.. one run with full tests
Added new disk_cleanup.py module providing comprehensive disk cleanup functionality for storage test teardown. This module handles:
Refactor 13 test files to use the new cleanup_disks() function:
Benefits: