mdaffin/loopdev

Make cargo-feature for `LOOP_SET_DIRECT_IO` support

Closed this issue · 4 comments

The new functionality was added in 0.2.2 : This commit introduces LOOP_SET_DIRECT_IO support. However, this IOCTL is absent on distros with 3.x kernels, like CentOS 7. Thus, we have a compilation error:

error[E0432]: unresolved import bindings::LOOP_SET_DIRECT_IO
--> /home/xxxxxxxxx/.cargo/registry/src/github.com-xxxxxxxxx/loopdev-0.2.2/src/lib.rs:40:69
|
40 | loop_info64, LOOP_CLR_FD, LOOP_CTL_GET_FREE, LOOP_SET_CAPACITY, LOOP_SET_DIRECT_IO,
| ^^^^^^^^^^^^^^^^^^ no LOOP_SET_DIRECT_IO in bindings

I suggest to add cargo-feature for such direct io functionality and to mark the function pub fn set_direct_io(&self, direct_io: bool) -> io::Result<()> { with such feature to compile it conditionally.

Created a PR to do this, though I have not tested it against Centos 7 yet. Can you let me know if that solves the issue. It is probably worth adding a new test target to catch these issues for older distros but this will require a bit more time for me to do.

I have defaulted the feature to on for now though I am not sure that is the right choice. @flxo what are your thoughts given you are likely the only user of this feature at this point.

flxo commented

I did't do any research when the direct IO feature has been added (or removed somewhere) and though it's a standard kernel/libc feature.

I'd be also fine with defaulting to off if it's not present on the majority of distribution.

I think it will be present in the majority of distributions, Centos 7 however has a very old kernel (some versions of debian might also suffer from this). I think defaulting it off still makes sense though as it means if some intermediary package does not need the feature then they don't need to worry about toggling it off for Centos 7. With it defaulted on then users need to explicitly turn it off if they want to support Centos 7.

@mdaffin Thank you! I've tested your branch on CentOS 7 box, it works well.