Skip to content
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

disk plugin: ignore loop devices #678

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

panlinux
Copy link

It doesn't make sense to apply tuning parameters to loop devices, since they point at files that live in other block devices, also subject to tuning.

I checked the device structure looking for a telltale of a loop device. I thought that the loop/backing_file attribute would be a good choice, but I had some that had no such attribute. I could also inspect the name of the device, and assume "loop*" is a loop device. I ended up picking the major number, 7, and was a bit annoyed that there was no straight attribute with that number, so there is some ugly parsing of the dev attribute.

Let's use this PR to plant the idea of ignoring loop devices. If that sounds ok and I didn't miss a use case where we want loop devices, we can iterate over the best way to do it.

Thanks!

@panlinux panlinux force-pushed the upstream-ignore-loop-devices branch 2 times, most recently from 63a36f4 to 20fd4f1 Compare August 21, 2024 20:25
It doesn't make sense to apply tuning parameters to loop devices, since they
point at files that live in other block devices, also subject to tuning.

Signed-off-by: Andreas Hasenack <[email protected]>
@panlinux panlinux force-pushed the upstream-ignore-loop-devices branch from 20fd4f1 to afbfb7d Compare September 6, 2024 19:12
@panlinux
Copy link
Author

panlinux commented Sep 6, 2024

Thanks to Athos Ribeiro, here is a much simpler version that I force-pushed.

@yarda
Copy link
Contributor

yarda commented Dec 11, 2024

Thanks. I think the change makes sense, but I am afraid it will break several Beaker functionality tests that use loop devices to safely test disk plugin features. That's why I would prefer it to be configurable, e.g. in /etc/tuned/tuned-main.conf and by default the loopdev tuning can be disabled. In such case if there is anybody who was using it they could easily re-enable it.

I think the loopdev can be detected if the device has the udev disk/by-loop* attribute(s), because IMHO if there is a loopdev there should be associated inode on the filesystem, so the disk/by-loop-inode should be there. But hardcoding the devid 7 is probably also OK.

@yarda
Copy link
Contributor

yarda commented Dec 11, 2024

Thanks. I think the change makes sense, but I am afraid it will break several Beaker functionality tests that use loop devices to safely test disk plugin features. That's why I would prefer it to be configurable, e.g. in /etc/tuned/tuned-main.conf and by default the loopdev tuning can be disabled. In such case if there is anybody who was using it they could easily re-enable it.

Or per plugin configuration when the #626 is merged.

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.

2 participants