Android build, additional flags, AsRawFd, IntoRawFd, CI

[flxo]

This PR contains a couple of things I'd need in loopdev:

• Build for Android: Androids bionic differs in some ways from glibc and others. The path to loop devices is different. Add a cfg directive for target_os = "android" and not.
• Add flags: read_only, autoclear and direct io
• Expose the loop devices Metadata

On the way there I added bindgen in a build.rs to gather the bindings during the build. Vagrant refused to work in my setup so I run the tests locally and added a GH actions script. The tests were flaky in GH because (as we all know) the loopdev driver sometimes lazy and just return an EBSY. I replace one of the losetup calls with a call from this crate - works so far.

@mdaffin Let me know which of the above things can be merged. I can prepare dedicated and clean PRs for each (and work with my master in the meantime ;-)). I did the GH actions stuff just because I'm curious....there's not need to that.

[mdaffin]

Hey, thanks for this.

I see you have disabled all the doc tests - I think these should be kept enabled to ensure the docs examples remain working. They suffer from the same issues as the integration tests so I don't see why they need to explicitly be disabled. You can always disable them locally by running only the integration tests with --test '*'. I think most of the examples could move away from explicitly specifying a device and use next_free instead which would elevate the collision issues you might be seeing.

I do wonder if the metadata is required? I see you are using it in the tests to get the minor number. Might be worth exposing a function for that purpose as appose to the full metadata. Do you have any other use cases for it? We could expose some of the other more useful info as explicit functions. It might be useful but not sure I want to commit to it as part of the API just yet if there is no other use case for it. You could always ask for the path and get the meta data from that. What other info do you require from it?

The rest seems good to me.

[mdaffin]

Looks like the tests are failing due to changing how sudo is used in the tests but not updating the travis.yaml to match: should be a matter of removing the sudo from https://github.com/mdaffin/loopdev/blob/master/.travis.yml#L34
Though if GH actions work then I will likely drop travis support as I am not a big fan of it (but at the time I wrote this travis was the best CI system that the tests would run in given they need kernel and root access to run). But it would be good to have them both working before we yank them out.

[flxo]

Hi @mdaffin. Thanks for your comments.

Regarding the doc tests: Still they need a sudo that's why I added the norun flags to be able to just do a cargo test in combination with the configured runner that adds a sudo when executing tests but not when doing doc tests.

I need the metadata for passing the minor to the device mapper during for device mapper verity tables. Getting the minor from the already opened File saves me a additional open call, just to obtain major/minor. Returning the full Metadata seemed reasonable to me since it's the same as in File and the recognition effect is bigger. I can change that to whatever you want.

Travis: yeah...same story everywhere that it was the defacto standard.

�I'll split this PR into a set of smaller ones:

• Android build: Make LOOP_PREFIX target_os specific
• Add AsRawFd IntoRawFd for LoopControl. Maybe also for LoopDevice? Add metadata to LoopDevice
• read_only auto clear and direct io
• bindgen in build.rs instead of hard coded constants
• GH actions

[flxo]

Split this PR into:

#28
#29
#30
#31

[mdaffin]

Regarding the doc tests: Still they need a sudo that's why I added the norun flags to be able to just do a cargo test in combination with the configured runner that adds a sudo when executing tests but not when doing doc tests.

That is annoying. Wonder if there is an equivalent for doc tests? I think keeping the doc tests working and compiling is the better thing to do though. This library does not need updating very often and it is better that the examples remain relevant than to make it more convenient to run the tests. It is a very annoying trade off to make though. But rather have that annoyance in this library then in places that use it.

I need the metadata for passing the minor to the device mapper during for device mapper verity tables. Getting the minor from the already opened File saves me a additional open call, just to obtain major/minor. Returning the full Metadata seemed reasonable to me since it's the same as in File and the recognition effect is bigger. I can change that to whatever you want.

You don't need an extra open call. You do need a call to stat, which you can do from a open fd or from a path.

Getting the minor number I think is could be a commonly needed thing that it is worth exposing directly allowing users to ld.minor_number().unwrap() or ld.minor().unwrap() or ld.nunber().unwrap() instead of

    let meta = ld.metadata().unwrap();
    let rdev = meta.rdev();
    let minor = ((rdev >> 12) & 0xFFFF_FF00) | (rdev & 0xFF);

Not sure how useful metadata is on loop devices once we have that? Does len show the length of the backing file (I would guess so)? Would permissions or access times be useful? Might be useful enough to include though not fully sure 🤔.

* GH actions

Did you miss this PR, I don't see it included in the others?

[flxo]

@mdaffin Thanks for your fast reviews and merges!

Regarding the doc tests: Still they need a sudo that's why I added the norun flags to be able to just do a cargo test in combination with the configured runner that adds a sudo when executing tests but not when doing doc tests.

That is annoying. Wonder if there is an equivalent for doc tests? I think keeping the doc tests working and compiling is the better thing to do though. This library does not need updating very often and it is better that the examples remain relevant than to make it more convenient to run the tests. It is a very annoying trade off to make though. But rather have that annoyance in this library then in places that use it.

Yes. I expected that the doc tests are executed with the configured runner - but that's not the case. Means currently the only solution is to sudo cargo test .... This is annoying locally because of smashed file ownerships etc... For the CI I think this is ok.

I need the metadata for passing the minor to the device mapper during for device mapper verity tables. Getting the minor from the already opened File saves me a additional open call, just to obtain major/minor. Returning the full Metadata seemed reasonable to me since it's the same as in File and the recognition effect is bigger. I can change that to whatever you want.

You don't need an extra open call. You do need a call to stat, which you can do from a open fd or from a path.

Getting the minor number I think is could be a commonly needed thing that it is worth exposing directly allowing users to ld.minor_number().unwrap() or ld.minor().unwrap() or ld.nunber().unwrap() instead of

    let meta = ld.metadata().unwrap();
    let rdev = meta.rdev();
    let minor = ((rdev >> 12) & 0xFFFF_FF00) | (rdev & 0xFF);

Not sure how useful metadata is on loop devices once we have that? Does len show the length of the backing file (I would guess so)? Would permissions or access times be useful? Might be useful enough to include though not fully sure 🤔.

You're right. If you really need the metadata you still have File::from_raw_fd - just dangerous because of the Drop impl. I can prepare a small pr that removes metadata and adds major and minor.

* GH actions

Did you miss this PR, I don't see it included in the others?

No - I skipped this for now. Turned out that the tests as they are today don't work in GH actions. The timings don't fit sometimes and the use of losetup results in EBUSY from time to time. Still the sudo thing from above also applies. I'll try to continue here.

[flxo]

Created #32

[mdaffin]

You're right. If you really need the metadata you still have File::from_raw_fd - just dangerous because of the Drop impl. I can prepare a small pr that removes metadata and adds major and minor.

You dont even need to do that. You can just ld.path().metadata().unwrap().

Did you miss this PR, I don't see it included in the others?

No - I skipped this for now. Turned out that the tests as they are today don't work in GH actions. The timings don't fit sometimes and the use of losetup results in EBUSY from time to time. Still the sudo thing from above also applies. I'll try to continue here.

That issue also exists on the travis tests, would be a really nice one to solve.

[flxo]

Opened a issue about the failing losetup calls #34 and documented the findings regarding the test execution in #35.

[flxo]

Closing this PR as all patches beside the CI topics are merged. The CI topics are documented in #34 and #35.