Skip to content

Incremental: do not call Manage_subdevs() from IncrementalRemove() #31

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
Merged
mtkaczyk merged 4 commits into from
Dec 16, 2024
Merged

Incremental: do not call Manage_subdevs() from IncrementalRemove() #31

mtkaczyk merged 4 commits into from
Dec 16, 2024

Conversation

@mtkaczyk
Copy link
Member

@mtkaczyk mtkaczyk commented Jun 21, 2024 •
edited
Loading

This is a first part of HOT_REMOVE_DISK/SET_FAULTY rework. The goal is to not execute HOT_REMOVE_DISK ioctl on mdadm --remove path with container.

This PR changes following:

  • Manage_subdevs will not execute HOT_REMOVE_IOCTL with "faulty" (not needed because sysfs way is same reliable).
  • Incremental_remove will implement remove logic directly.

@mtkaczyk mtkaczyk marked this pull request as draft June 25, 2024 08:55
@mtkaczyk mtkaczyk force-pushed the fix_long_remove_external branch from 83fc6b2 to 12989b5 Compare June 25, 2024 13:39
@mtkaczyk mtkaczyk changed the title Manage: Rework external Manage_remove() path Incremental: do not call Manage_subdevs() from IncrementalRemove() Jun 25, 2024
@mtkaczyk mtkaczyk marked this pull request as ready for review June 27, 2024 14:11
bkucman
bkucman requested changes Jul 2, 2024
View reviewed changes
Copy link
Collaborator

@bkucman bkucman left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi,
Overall I agree with the change, I have a few minor questions or corrections.

Typo in commit desc ee89efa
dediated -> dedicated

Unnecessary dots in commit titles

@mtkaczyk
Copy link
Member Author

mtkaczyk commented Jul 5, 2024

@bkucman
thanks for review, For now moving again to draft, I opened new PR #46 - it is a preparation for this change and needs to come first.

@mtkaczyk mtkaczyk marked this pull request as draft July 5, 2024 09:12
@mtkaczyk
Copy link
Member Author

mtkaczyk commented Sep 4, 2024

Part of this (support for devnode in mdadm -If) is moved to #87
After merging #87 I will get back to this PR.

@mtkaczyk mtkaczyk force-pushed the fix_long_remove_external branch 2 times, most recently from 456d4f0 to 2b121df Compare September 10, 2024 12:08
@mtkaczyk mtkaczyk force-pushed the fix_long_remove_external branch from 2b121df to e25f104 Compare November 5, 2024 12:10
@mtkaczyk mtkaczyk marked this pull request as ready for review November 5, 2024 12:13
@mtkaczyk mtkaczyk force-pushed the fix_long_remove_external branch from c875c84 to 1c146da Compare November 22, 2024 08:35
@mtkaczyk
Incremental: Document workaround
77fd7dd 
Keep it documented in code.

Signed-off-by: Mariusz Tkaczyk <[email protected]>
@mtkaczyk
sysfs: functions for writing md/<memb>/state
493bc0e 
Add dedicated enum to reflect possible values for mentioned file.
Not all values are mapped. Add map to present sysfs keywords.

Signed-off-by: Mariusz Tkaczyk <[email protected]>
@mtkaczyk
Incremental: Simplify remove logic
c1e05d1 
Incremental_remove() does not execute Manage_subdevs() now.

Signed-off-by: Mariusz Tkaczyk <[email protected]>
@mtkaczyk
checkpatch.conf: ignore NEW_TYPEDEFS
1c146da 
In mdadm, we have more flexible apporach to typedefs.

Signed-off-by: Mariusz Tkaczyk <[email protected]>
@ncroxon
Copy link
Collaborator

ncroxon commented Nov 26, 2024

Nice change/cleanup. Do we have any tests in place to verify functionality change?

@mtkaczyk
Copy link
Member Author

mtkaczyk commented Nov 27, 2024

I tested it functionally on my own with native and IMSM. No regression detected but it was minimal testing.
I will merge it after the release. I'm waiting for Kuai patches for bitmap right now and then I will make a release.

You can see testing result here:
https://github.com/md-raid-utilities/mdadm/actions/runs/11969166801/job/33369355291?pr=31
I see no new failure but I will look closer later.

@mtkaczyk
Copy link
Member Author

mtkaczyk commented Nov 29, 2024

@ncroxon, comparing to Nightly execution on the latest main. I see 01r5fail fixed probably by this PR.

05r1-internalbitmap-v1c is broken but this functionality and test will be removed soon.

@mtkaczyk mtkaczyk merged commit 143d94f into md-raid-utilities:main Dec 16, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bkucman bkucman Awaiting requested review from bkucman

@ktanska ktanska Awaiting requested review from ktanska

@mku514k mku514k Awaiting requested review from mku514k

@asztukow asztukow Awaiting requested review from asztukow

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mtkaczyk @ncroxon @bkucman