Skip to content

Support parse raid type for mdstat#505

Open
ImSingee wants to merge 4 commits into
prometheus:masterfrom
ImSingee:master
Open

Support parse raid type for mdstat#505
ImSingee wants to merge 4 commits into
prometheus:masterfrom
ImSingee:master

Conversation

@ImSingee

Copy link
Copy Markdown
Contributor

For something like

md6 : active raid1 sdb2[2](F) sdc[1](S) sda2[0]
md11 : active (auto-read-only) raid1 sdb2[0] sdc2[1] sdc3[2](F) hda[4](S) ssdc2[3](S)

We can also get the raid type from /proc/mdstat

Comment thread mdstat.go Outdated
Comment thread mdstat_test.go Outdated
Comment thread mdstat.go Outdated
@SuperQ

SuperQ commented Apr 13, 2023

Copy link
Copy Markdown
Member

This needs a DCO sign-off. You can use git commit -s --amend to add it.

@ImSingee

Copy link
Copy Markdown
Contributor Author

@SuperQ Sorry that I forgot the sign-off in later commits😂. And now it's added.

@SuperQ SuperQ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original checking for (...) in the word after active/inactive was better.

Comment thread mdstat.go Outdated
}

func isRaidType(mdType string) bool {
return strings.HasPrefix(mdType, "raid") || mdType == "linear"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think this is going to work. Like I said, there are a whole bunch of other types other than raid.... Since this can change depending on the kernel/mdadm version. We can't support an exhaustive list here.

Comment thread mdstat.go Outdated
state := deviceFields[2] // active or inactive

mdType := "unknown"
if len(deviceFields) > 3 && isRaidType(deviceFields[3]) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you move this to after if len(lines) <= i+3 { you can reduce the word checking to just the check for (...).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, maybe this whole piece should be moved to the evalStatusLine() function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

len(lines) <= i+3 is the guard for lines not the fields in first line, so the move won't be helpful

@ImSingee

ImSingee commented Apr 13, 2023

Copy link
Copy Markdown
Contributor Author

@SuperQ There's a case that I cannot check (...) only

md219 : inactive sdb[2](S) sdc[1](S) sda[0](S)

If we only check parentheses, the type will be sdb[2](S)

But maybe we can assume the type does not contain any special characters?

@ImSingee

Copy link
Copy Markdown
Contributor Author

@SuperQ I changed the code for type checking. It passes all current tests, and I hope it can work forever.

@SuperQ SuperQ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we only get a type if state is active. Maybe check that first before checking the next word or two.

@ImSingee

Copy link
Copy Markdown
Contributor Author

@SuperQ I considered this before. But there is another case...

md4 : inactive raid1 sda3[0](F) sdb3[1](S)
      4883648 blocks [2/2] [UU]

We can ignore the type for this, but it's here anyway and it may be useful in some case.

@SuperQ

SuperQ commented Apr 13, 2023

Copy link
Copy Markdown
Member

Ahh, crap, yea, that's a problem.

Comment thread mdstat.go
// check if a string's format is like the mdType
// Rule 1: mdType should not be like (...)
// Rule 2: mdType should not be like sda[0]
func isRaidType(mdType string) bool {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ewww hah..
There must be a better way. Is there an explicit way to query the md type? E.g by reasing another procfs file?

@ImSingee

Copy link
Copy Markdown
Contributor Author

@SuperQ @discordianfish Because of the lack of some knowledge, let's keep this open and see if anyone can help.

@robbat2

robbat2 commented Oct 3, 2024

Copy link
Copy Markdown
Contributor

@ImSingee please see my PR #674 that fixes the outstanding issues here; but your commit are missing some signed-off tags - could you please fix that in this PR and i'll rebase my PR on top of that.

@ImSingee

ImSingee commented Oct 3, 2024

Copy link
Copy Markdown
Contributor Author

@robbat2 Sure, I will update this later this day!

Signed-off-by: Singee <git@singee.me>
Signed-off-by: Singee <git@singee.me>
Signed-off-by: Singee <git@singee.me>
Signed-off-by: Singee <git@singee.me>
@ImSingee

ImSingee commented Oct 3, 2024

Copy link
Copy Markdown
Contributor Author

@robbat2 I have added the Sign-Off

@robbat2

robbat2 commented Oct 3, 2024

Copy link
Copy Markdown
Contributor

@robbat2 I have added the Sign-Off

Thank you; rebased my PR now

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.

4 participants