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

Add a new create_options argument to new() and setup() #848

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bmr-cymru
Copy link
Member

This branch adds the ability for callers to control the DmOptions used when creating and setting up devices. This gives users of the crate control over the udev flags used when the DM device is set up, allowing control of device visibility and flag affecting udev rule behaviour:

       create_options          devicemapper-rs device behavior
+----------------------------+------------------------------------------------+
| None                       | Accept historical library defaults.            |
+----------------------------+------------------------------------------------+
| Some(DmOptions::default()) | Library defaults for visible devices.          |
+----------------------------+------------------------------------------------+
| Some(DmOptions::private()) | Library defaults for private (hidden) devices. |
+----------------------------+------------------------------------------------+
| Some(custom_options)       | Caller has complete control of options/flags.  |
+----------------------------+------------------------------------------------+

@bmr-cymru bmr-cymru changed the title Add a new create_options argument to new() and setup() methods Add a new create_options argument to new() and setup() Apr 6, 2023
@bmr-cymru
Copy link
Member Author

bmr-cymru commented Apr 6, 2023

This is failing the clippy job on Ubuntu 20.04:

    Checking devicemapper v0.33.3 (/root/src/git/devicemapper-rs)
error: multiple versions for dependency `windows-sys`: 0.45.0, 0.48.0
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions
  = note: `-D clippy::multiple-crate-versions` implied by `-D clippy::cargo`

error: multiple versions for dependency `windows-targets`: 0.42.2, 0.48.0
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions

error: multiple versions for dependency `windows_aarch64_gnullvm`: 0.42.2, 0.48.0
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions

error: multiple versions for dependency `windows_aarch64_msvc`: 0.42.2, 0.48.0
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions

error: multiple versions for dependency `windows_i686_gnu`: 0.42.2, 0.48.0
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions

error: multiple versions for dependency `windows_i686_msvc`: 0.42.2, 0.48.0
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions

error: multiple versions for dependency `windows_x86_64_gnu`: 0.42.2, 0.48.0
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions

error: multiple versions for dependency `windows_x86_64_gnullvm`: 0.42.2, 0.48.0
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions

error: multiple versions for dependency `windows_x86_64_msvc`: 0.42.2, 0.48.0
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions

This seems to happen because e.g. io-lifetimes depends on windows-sys 0.48.0 while is-terminal wants 0.45.0 etc. It's not clear to me why this is only happening on Ubuntu - "make clippy" on Fedora 37 passes with the same Cargo.toml.

This was reported upstream last year. There doesn't seem to be a resolution in the issue, but there is a comment that this lint possibly doesn't belong in clippy::cargo:

"There's nothing immediately you can do about it, it's not really a generally applicable lint"

The only way I've found to get this to pass on Ubuntu 20.04 is to explicitly disable the lint with -A:

# cargo clippy --all-targets --all-features -- -D clippy::cargo -D clippy::all -A clippy::multiple-crate-versions
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s

@mulkieran
Copy link
Member

The Ubuntu/Fedora discrepancy is odd. If I run cargo duplicates on my machine now, without your changes, it lists all those windows crates as duplicates. But clippy doesn't see them, as you say.

@bmr-cymru
Copy link
Member Author

This needs additional changes to also expose the visibility control to clients on resume of a DM device.

Add the ability for callers to specify the DmOptions value to use
when creating or setting up lineardev, thindev, thinpooldev, and
cachedev.

The DmOptions value is wrapped in an Option which if set to None
gives the previous devicemapper-rs default behaviour.

This allows clients of the crate to opt in to controlling device
visibility and other DmOptions/DmUdevFlags behaviour when creating
or setting up devices.
Allow callers to control DmOptions and udev flags on resume. This
allows clients of the crate to explicitly control the flags used
on device resume.

If no DmOptions is supplied to a call the library reverts to the
long-standing default behaviour.
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