mdstat_ent handling rework#280
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors /proc/mdstat parsing and related helpers by moving the mdstat_ent API out of mdadm.h into a dedicated header (mdstat.h), and centralizing the "external:" prefix into a shared constants header (md_common.h). The intent aligns with the stated goal of decentralizing mdadm internals without functional changes.
Changes:
- Introduces
mdstat.h(mdstat API/types) andmd_common.h(shared md-driver string constants). - Removes
mdstat_entand mdstat function declarations frommdadm.h, updating call sites to includemdstat.h. - Replaces hard-coded
"external:"and numeric offsets withMD_EXTERNAL_PREFIX/MD_EXTERNAL_PREFIX_LENin multiple parsing/formatting paths, and relocatesis_subarray_active()intomdstat.c.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| util.c | Removes is_subarray_active() implementation (moved to mdstat module). |
| sysfs.c | Uses shared "external:" prefix macros when parsing/writing sysfs metadata version. |
| super-intel.c | Updates external-metadata parsing offsets to use MD_EXTERNAL_PREFIX_LEN; adds mdstat.h include. |
| msg.c | Adds mdstat.h include for mdstat types/helpers. |
| mdstat.h | New header exposing mdstat_ent type and mdstat helper APIs. |
| mdstat.c | Includes mdstat.h, uses shared prefix macros, and hosts is_subarray_active(). |
| mdopen.c | Adds mdstat.h include for mdstat types/helpers. |
| mdmonitor.c | Adds mdstat.h include and updates external-metadata offsets to MD_EXTERNAL_PREFIX_LEN. |
| mdmon.h | Removes mdstat declarations now owned by mdstat.h. |
| mdmon.c | Adds mdstat.h include for mdstat types/helpers. |
| mdadm.h | Removes mdstat_ent struct and mdstat-related declarations and inline helper. |
| mdadm.c | Adds mdstat.h include for mdstat types/helpers. |
| md_common.h | New header centralizing shared md-driver constants (e.g., MD_EXTERNAL_PREFIX). |
| mapfile.c | Adds mdstat.h include for mdstat types/helpers. |
| managemon.c | Adds mdstat.h include for mdstat types/helpers. |
| Manage.c | Adds mdstat.h include and uses MD_EXTERNAL_PREFIX for metadata version composition. |
| Kill.c | Adds mdstat.h include for is_subarray_active() usage. |
| Incremental.c | Adds mdstat.h include for mdstat types/helpers. |
| Grow.c | Adds mdstat.h include and updates external-metadata offset usage. |
| Detail.c | Adds mdstat.h include and updates external-metadata parsing to use shared prefix macros. |
| config.c | Adds mdstat.h include for mdstat types/helpers. |
| Assemble.c | Adds mdstat.h include and updates external-metadata offset usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3a356f3 to
e6a5ddf
Compare
Move mdstat_ent handling to separate header and include this header directly. No functional changes are intended. Signed-off-by: Mariusz Tkaczyk <mtkaczyk@kernel.org>
920e4dd to
32dda1f
Compare
bkucman
left a comment
There was a problem hiding this comment.
Hi, thank you for your patch.
there are still two more spot with "external:", you could also use define, unless you think that's overkill.
mdadm.c:1857
int member = e->metadata_version &&
strncmp(e->metadata_version,
"external:/", 10) == 0;
monitor.c:495
if (strncmp(buf, "external:-", 10) == 0) {
To have it better understandable add definitions for that. Definition is placed in md_common.h because it is corresponding to both mdstat and sysfs implementations. Probably more shared properties will be defined in the future. The MD_EXTERNAL_PREFIX_LEN and MD_VERSION_BLOCKED_IDX are the same but are defined for different purpose. One is specyfing size of string but the second is specyfing index in the array. Also, replaces magic "external" used in code. No functional changes intended. Signed-off-by: Mariusz Tkaczyk <mtkaczyk@kernel.org>
lets proceed with this smaller rework. What you are asking is complete rework of how to set and read the version which I don't have good concept yet. Noted this as a another thing to rework. |
No functional changes, just a follow-up of previous actions with a goal of decentralizing mdadm.