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

Support parse raid type for mdstat #505

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ImSingee
Copy link

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

mdstat.go Outdated Show resolved Hide resolved
mdstat_test.go Outdated Show resolved Hide resolved
mdstat.go Outdated Show resolved Hide resolved
@SuperQ
Copy link
Member

SuperQ commented Apr 13, 2023

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

@ImSingee
Copy link
Author

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

Copy link
Member

@SuperQ SuperQ left a comment

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.

mdstat.go Outdated
@@ -165,6 +175,10 @@ func parseMDStat(mdStatData []byte) ([]MDStat, error) {
return mdStats, nil
}

func isRaidType(mdType string) bool {
return strings.HasPrefix(mdType, "raid") || mdType == "linear"
Copy link
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.

mdstat.go Outdated
@@ -95,6 +97,13 @@ func parseMDStat(mdStatData []byte) ([]MDStat, error) {
mdName := deviceFields[0] // mdx
state := deviceFields[2] // active or inactive

mdType := "unknown"
if len(deviceFields) > 3 && isRaidType(deviceFields[3]) {
Copy link
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
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
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
Copy link
Author

ImSingee commented Apr 13, 2023

@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
Author

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

Copy link
Member

@SuperQ SuperQ left a comment

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
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
Copy link
Member

SuperQ commented Apr 13, 2023

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

// 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
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
Author

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

@robbat2
Copy link

robbat2 commented Oct 3, 2024

@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
Copy link
Author

ImSingee commented Oct 3, 2024

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

@ImSingee
Copy link
Author

ImSingee commented Oct 3, 2024

@robbat2 I have added the Sign-Off

@robbat2
Copy link

robbat2 commented Oct 3, 2024

@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