resource-agents
resource-agents copied to clipboard
storage_mon: Use the O_DIRECT flag in open() to eliminate cache effects
Use of O_DIRECT in open() eliminates the possibility of false negatives resulting from buffer cache reads.
please refer to https://lists.clusterlabs.org/pipermail/users/2022-August/030459.html for background.
Can one of the admins verify this patch?
test this please
I agree with Ken (in the above thread) that there should be a fallback if O_DIRECT fails. There are filesystems and options even on Linux that don't always support O_DIRECT and they shouldn't be ecxluded or errored
- https://github.com/ClusterLabs/resource-agents/pull/1797/commits/ce4e632f29ed6b86b82a959eac5844655baed153
- According to https://ci.kronosnet.org/job/resource-agents-build-all-voting/1530/, build targets are FreeBSD and RHEL etc., so I deleted
#if defined(__linux__) || defined(__FreeBSD__)
. - Fixed FreeBSD build failure.
- According to https://ci.kronosnet.org/job/resource-agents-build-all-voting/1530/, build targets are FreeBSD and RHEL etc., so I deleted
- The open() manual says it returns EINVAL for filesystems that do not support O_DIRECT. Are there any issues that the implementation below does not address?
- https://github.com/ClusterLabs/resource-agents/pull/1797/files#diff-91e236cc5ccce26f5a934e98aef5f1082251a8676e999b5364add5603538483dR44
Also, I don't understand why the error checking for the ioctl is done on the sec_size value rather than the ioctl return. If that's fine then a comment saying why would be helpful. Maybe we need both?
Some kind of shortcut for 'ioctl has to succeed and not return bogus'. But some comment would of course make sense. Basically I guess the code should be safe though I don't know if there are actually cases where ioctl would succeed without returning a proper block-size. Same code-pattern btw. has been in sbd since ever.
- https://github.com/ClusterLabs/resource-agents/pull/1797/commits/7a0aaa0dfdebeab3fae9fe9ddc412c3d1f610273
- reflected the https://github.com/ClusterLabs/resource-agents/pull/1797#pullrequestreview-1080084034.
retest this please
retest this please
Thanks.