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 support for /dev/disk/by-id/.... notation in config #92

Closed
wants to merge 2 commits into from

Conversation

func0der
Copy link

Replaces #56
Relates to #30

Couple of remarks:
This script has multiple places where drives and partitions are handled. Almost all of them need to be aware of the
correct notation of partitions.
I think, that most of the time it is just a lucky hit, that it works.
Also the part where it either uses p (nvme) or -part is in there at least twice, so parts of that script have been compatible with the by-id notation already.

I do not feel comfortable doing that kind of refactoring, though. Just wanted to mention that it might be a good idea to pull partition detection by name (or something) together some place.

Either way, I think this is a good start.

ableischwitz and others added 2 commits November 15, 2023 00:04
This change introduces the detection of `DRIVEn /dev/disk/by-id/xxxx' devices.
Additionally to the different nvme naming for partitions, those disks get a `-partx` added to the device - instead of a single number
Syntax for '/dev/disk/by-id' has '-partX' for the partition behind the
drive paths. So we need to replace them here too, if found. Else the
script would try to format the underlying drive and not the raid array,
which would not work, because after joining the raid array the drive is
constantly busy.
@func0der
Copy link
Author

func0der commented Dec 5, 2023

@asciiprod LGTM? :)

@func0der
Copy link
Author

func0der commented Feb 2, 2024

Hello?

@asciiprod
Copy link
Collaborator

There was another place which had to be adjusted, but in general it should work now.

@asciiprod asciiprod closed this Apr 15, 2024
@func0der
Copy link
Author

Thanks for merging.

I just realized that I forgot to tick 'allow edits by maintainer' and wondered why the PR was not merged but rather the contents copied.

Anyway: Glad the repo is alive and this feature made it in

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

3 participants