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
@mulkieran mulkieran added this to In progress (long term) in 2023April via automation 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.
@mulkieran mulkieran removed this from In progress (long term) in 2023April May 1, 2023
@mulkieran mulkieran added this to In progress (long term) in 2023May via automation May 1, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023May Jun 5, 2023
@mulkieran mulkieran added this to In progress (long term) in 2023June via automation Jun 5, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023June Jul 10, 2023
@mulkieran mulkieran added this to In progress (long term) in 2023July via automation Jul 10, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023July Aug 7, 2023
@mulkieran mulkieran added this to In progress (long term) in 2023August via automation Aug 7, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023August Sep 5, 2023
@mulkieran mulkieran added this to In progress (long term) in 2023September via automation Sep 5, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023September Oct 3, 2023
@mulkieran mulkieran added this to In progress (long term) in 2023October via automation Oct 3, 2023
@mulkieran mulkieran moved this from In progress (long term) to To do in 2023October Oct 16, 2023
@mulkieran mulkieran moved this from To do to In progress (long term) in 2023October Oct 16, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023October Oct 30, 2023
@mulkieran mulkieran added this to In progress (long term) in 2023November via automation Oct 30, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023November Dec 4, 2023
@mulkieran mulkieran added this to In progress (long term) in 2023December via automation Dec 4, 2023
@mulkieran mulkieran removed this from In progress (long term) in 2023December Jan 2, 2024
@mulkieran mulkieran added this to In progress (long term) in 2024January via automation Jan 2, 2024
@mulkieran mulkieran removed this from In progress (long term) in 2024January Feb 5, 2024
@mulkieran mulkieran added this to In progress (long term) in 2024February via automation Feb 5, 2024
@mulkieran mulkieran removed this from In progress (long term) in 2024February Mar 5, 2024
@mulkieran mulkieran added this to In progress (long term) in 2024March via automation Mar 5, 2024
@mulkieran mulkieran removed this from In progress (long term) in 2024March Apr 2, 2024
@mulkieran mulkieran added this to In progress (long term) in 2024April via automation Apr 2, 2024
@mulkieran mulkieran removed this from In progress (long term) in 2024April Apr 29, 2024
@mulkieran mulkieran added this to In progress (long term) in 2024May via automation Apr 29, 2024
@mulkieran mulkieran removed this from In progress (long term) in 2024May May 28, 2024
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.

None yet

2 participants