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.

Sign up for GitHub

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

Jump to bottom

Unify device discovery on initial start up and event-driven paths #1767

Merged
merged 11 commits into from
Jan 7, 2020
Merged

Unify device discovery on initial start up and event-driven paths #1767

merged 11 commits into from
Jan 7, 2020

Conversation

mulkieran
Copy link
Member

@mulkieran mulkieran commented Jan 3, 2020 •
edited
Loading

The major contributions of this PR are:

  • Make device identification via udev on event-driven path constant time, rather than linear in the number of block devices.
  • Establish a separate module for device identification and put all code for this, whether used on initial start up or event-driven in this module.
  • Unify code for event-driven and initial setup discovery. The overall behavior was more or less the same, but now the shared behavior has shared code and shared logging.
  • Move some stray bits of engine actions out of bin/stratisd.rs. This is just generally good design; it also enabled the change from linear to constant time.
  • Make all device discovery intended to discover Stratis devices for the purpose of setting up pools
    more or less uniform.

Resolves: #1732.
Related: #1659.

@mulkieran mulkieran self-assigned this Jan 3, 2020
@mulkieran mulkieran force-pushed the develop-block-evaluate branch 4 times, most recently from d369fbd to f3de73a Compare January 6, 2020 17:37
@mulkieran mulkieran changed the title Develop block evaluate Unify device discovery on initial start up and event-driven paths Jan 6, 2020
@mulkieran mulkieran force-pushed the develop-block-evaluate branch 5 times, most recently from d016dbd to ff461bf Compare January 7, 2020 14:52
@mulkieran
Move some engine-specific actions out of bin/stratisd.rs
a16170c 
Do this by making a new method, handle_event, which is part of the
Engine API and making block_evaluate private.

git is wierd, and rather than show the move of block_evaluate, it chooses
instead to show a move of create_pool, which wasn't really the moved
method.

Signed-off-by: mulhern <[email protected]>
@mulkieran
Remove dependency in tests on block_device_ownership
ec4c44a 
It's somewhat inappropriate, because block_device_ownership is
especially designed to be only called if a udev event on the device has
occurred and this is not what is occurring in the test.

Signed-off-by: mulhern <[email protected]>
@mulkieran
Explode find_all into some separate functions
b1e6195 
Signed-off-by: mulhern <[email protected]>
@mulkieran
Move all device identification into a separate module
9b67387 
Signed-off-by: mulhern <[email protected]>
@mulkieran
Have device_identifiers_wrapper return both device identifiers
8d0c4a4 
The device UUID is useful in some contexts soon to be introduced.

Signed-off-by: mulhern <[email protected]>
@mulkieran mulkieran force-pushed the develop-block-evaluate branch from ff461bf to 7a20f6b Compare January 7, 2020 20:40
@mulkieran
Copy link
Member Author

rebased.

@mulkieran
Introduce a new method, identify_block_device, in the identify module
8606697 
Use it in block_evaluate and discard any newly introduced dead code.
Have block_evaluate take a udev device as an argument.

This makes the udev lookup constant time instead of linear in the number
of block devices. Previously, the device node was discovered from the
udev database entry, and then the entries for all block devices in the udev
database were searched in order to locate a block device that had the
specified device node in order to locate the relevant udev database entry.

Simplify block_evaluate's return type. It never returns an error, logging any
internal errors instead.

Signed-off-by: mulhern <[email protected]>
@mulkieran
Abstract the action of checking a device after udev processing
a7ad092 
Use it in all methods where this makes sense, according to the context.

Fix a small bug in find_all_block_devices_with_stratis_signatures.
Account for the somewhat slight possibility that udev has belatedly
identified the device as a Stratis device.

Signed-off-by: mulhern <[email protected]>
@mulkieran
Make is_multipath_member private
d7b1b01 
Rely solely on publicly exposed decide_ownership method.

It allows for better encapsulation, it's a bit clearer and it makes
the actions that occur for the stratis enumeration and for the block
device enumeration more consistent.

Signed-off-by: mulhern <[email protected]>
@mulkieran
Have block_evaluate return the pool and the pool UUID
8294543 
This allows moving an expect to a position directly after the change that
makes it logically necessary, rather than in another method entirely.

Signed-off-by: mulhern <[email protected]>
@mulkieran
Use identify_block_device in the body of a find method
5899960 
Use it in the body of find_all_block_devices_with_stratis_signatures.
The context in which this method is called by block_evaluate is the same
as is established by find_all_block_devices_with_stratis_signatures.

Signed-off-by: mulhern <[email protected]>
@mulkieran
Make a method called identify_stratis_device
257c4d5 
This method has no public use yet, but is analogous to the
identify_block_device method. Use this method in
find_all_stratis_devices in the same way that identify_block_device is
used in find_all_block_devices_with_stratis_signatures.

Signed-off-by: mulhern <[email protected]>
@mulkieran mulkieran force-pushed the develop-block-evaluate branch from 7a20f6b to 257c4d5 Compare January 7, 2020 22:36
@mulkieran
Copy link
Member Author

There was a tiny clippy error that was in 1.40, but not in beta. I just fixed it since it was obviously correct. I filed a bug against clippy:
rust-lang/rust-clippy#5014.

@mulkieran mulkieran merged commit 2ab59b9 into stratis-storage:develop Jan 7, 2020
@mulkieran mulkieran deleted the develop-block-evaluate branch January 7, 2020 22:58
This was referenced Jan 7, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drckeefe drckeefe drckeefe approved these changes

@bgurney-rh bgurney-rh bgurney-rh approved these changes

@jbaublitz jbaublitz jbaublitz approved these changes

Assignees

@mulkieran mulkieran

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

udev-based part of device discovery mechanism silently skips uninitialized devices
4 participants
@mulkieran @jbaublitz @bgurney-rh @drckeefe