Closed zacikpa closed 4 months ago
Thank you for the PR, @zacikpa ! I tested it and it works as intended. I like this solution, especially the "caching" of hdparm's support for a block device. A couple of nits:
[disk]
plug-in, rather than Video plug-in.hdparm_apm_devices
with boolean values vs. two sets for supported/unsupported devices? This is an implementation detail and I guess it is a performance vs. readability issue. If the performance hit using associative array is not too high, it could help readability and get rid of one data structure.I'd definitely love to see this in the upcoming FDP 24.D (June 5th deadline) release, @yarda .
Thanks for the review, @jmencak. I updated the description with the correct plugin name and swapped the two sets for a dictionary.
Thank you for the changes, Pavol. LGTM and I've also retested this.
LGTM, thanks.
The disk plugin checks each device for hdparm support during initialization. When using many disk devices, this can lead to unwanted delays in profile application.
This change makes the hdparm check lazy by postponing it to the moment when hdparm is actually needed, i.e., during dynamic tuning or when setting spindown/apm. Each device is checked at most once - the plugin now stores the sets of hdparm-supported devices and hdparm-unsupported devices).
Resolves: RHEL-6891