On 22/08/2019 13:13, Guilherme G. Piccoli wrote: > Currently if a md raid0/linear array gets one or more members removed while > being mounted, kernel keeps showing state 'clean' in the 'array_state' > sysfs attribute. Despite udev signaling the member device is gone, 'mdadm' > cannot issue the STOP_ARRAY ioctl successfully, given the array is mounted. > > Nothing else hints that something is wrong (except that the removed devices > don't show properly in the output of mdadm 'detail' command). There is no > other property to be checked, and if user is not performing reads/writes > to the array, even kernel log is quiet and doesn't give a clue about the > missing member. > > This patch is the mdadm counterpart of kernel new array state 'broken'. > The 'broken' state mimics the state 'clean' in every aspect, being useful > only to distinguish if an array has some member missing. All necessary > paths in mdadm were changed to deal with 'broken' state, and in case the > tool runs in a kernel that is not updated, it'll work normally, i.e., it > doesn't require the 'broken' state in order to work. > Also, this patch changes the way the array state is showed in the 'detail' > command (for raid0/linear only) - now it takes the 'array_state' sysfs > attribute into account instead of only rely in the MD_SB_CLEAN flag. > > Cc: NeilBrown <neilb@xxxxxxxx> > Cc: Song Liu <songliubraving@xxxxxx> > Signed-off-by: Guilherme G. Piccoli <gpiccoli@xxxxxxxxxxxxx> > --- > > v2 -> v3: > * Nothing changed. > > v1 -> v2: > * Added handling for md/linear 'broken' state. > > > Detail.c | 17 +++++++++++++++-- > Monitor.c | 9 +++++++-- > maps.c | 1 + > mdadm.h | 1 + > mdmon.h | 2 +- > monitor.c | 4 ++-- > 6 files changed, 27 insertions(+), 7 deletions(-) > > diff --git a/Detail.c b/Detail.c > index ad60434..cc7e9f1 100644 > --- a/Detail.c > +++ b/Detail.c > @@ -81,6 +81,7 @@ int Detail(char *dev, struct context *c) > int external; > int inactive; > int is_container = 0; > + char arrayst[12] = { 0 }; /* no state is >10 chars currently */ > > if (fd < 0) { > pr_err("cannot open %s: %s\n", > @@ -485,9 +486,21 @@ int Detail(char *dev, struct context *c) > else > st = ", degraded"; > > + if (array.state & (1 << MD_SB_CLEAN)) { > + if ((array.level == 0) || > + (array.level == LEVEL_LINEAR)) > + strncpy(arrayst, > + map_num(sysfs_array_states, > + sra->array_state), > + sizeof(arrayst)-1); > + else > + strncpy(arrayst, "clean", > + sizeof(arrayst)-1); > + } else > + strncpy(arrayst, "active", sizeof(arrayst)-1); > + > printf(" State : %s%s%s%s%s%s \n", > - (array.state & (1 << MD_SB_CLEAN)) ? > - "clean" : "active", st, > + arrayst, st, > (!e || (e->percent < 0 && > e->percent != RESYNC_PENDING && > e->percent != RESYNC_DELAYED)) ? > diff --git a/Monitor.c b/Monitor.c > index 036103f..9fd5406 100644 > --- a/Monitor.c > +++ b/Monitor.c > @@ -1055,8 +1055,12 @@ int Wait(char *dev) > } > } > > +/* The state "broken" is used only for RAID0/LINEAR - it's the same as > + * "clean", but used in case the array has one or more members missing. > + */ > +#define CLEAN_STATES_LAST_POS 5 > static char *clean_states[] = { > - "clear", "inactive", "readonly", "read-auto", "clean", NULL }; > + "clear", "inactive", "readonly", "read-auto", "clean", "broken", NULL }; > > int WaitClean(char *dev, int verbose) > { > @@ -1116,7 +1120,8 @@ int WaitClean(char *dev, int verbose) > rv = read(state_fd, buf, sizeof(buf)); > if (rv < 0) > break; > - if (sysfs_match_word(buf, clean_states) <= 4) > + if (sysfs_match_word(buf, clean_states) > + <= CLEAN_STATES_LAST_POS) > break; > rv = sysfs_wait(state_fd, &delay); > if (rv < 0 && errno != EINTR) > diff --git a/maps.c b/maps.c > index 02a0474..49b7f2c 100644 > --- a/maps.c > +++ b/maps.c > @@ -150,6 +150,7 @@ mapping_t sysfs_array_states[] = { > { "read-auto", ARRAY_READ_AUTO }, > { "clean", ARRAY_CLEAN }, > { "write-pending", ARRAY_WRITE_PENDING }, > + { "broken", ARRAY_BROKEN }, > { NULL, ARRAY_UNKNOWN_STATE } > }; > > diff --git a/mdadm.h b/mdadm.h > index 43b07d5..c88ceab 100644 > --- a/mdadm.h > +++ b/mdadm.h > @@ -373,6 +373,7 @@ struct mdinfo { > ARRAY_ACTIVE, > ARRAY_WRITE_PENDING, > ARRAY_ACTIVE_IDLE, > + ARRAY_BROKEN, > ARRAY_UNKNOWN_STATE, > } array_state; > struct md_bb bb; > diff --git a/mdmon.h b/mdmon.h > index 818367c..b3d72ac 100644 > --- a/mdmon.h > +++ b/mdmon.h > @@ -21,7 +21,7 @@ > extern const char Name[]; > > enum array_state { clear, inactive, suspended, readonly, read_auto, > - clean, active, write_pending, active_idle, bad_word}; > + clean, active, write_pending, active_idle, broken, bad_word}; > > enum sync_action { idle, reshape, resync, recover, check, repair, bad_action }; > > diff --git a/monitor.c b/monitor.c > index 81537ed..e0d3be6 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -26,7 +26,7 @@ > > static char *array_states[] = { > "clear", "inactive", "suspended", "readonly", "read-auto", > - "clean", "active", "write-pending", "active-idle", NULL }; > + "clean", "active", "write-pending", "active-idle", "broken", NULL }; > static char *sync_actions[] = { > "idle", "reshape", "resync", "recover", "check", "repair", NULL > }; > @@ -476,7 +476,7 @@ static int read_and_act(struct active_array *a, fd_set *fds) > a->next_state = clean; > ret |= ARRAY_DIRTY; > } > - if (a->curr_state == clean) { > + if ((a->curr_state == clean) || (a->curr_state == broken)) { > a->container->ss->set_array_state(a, 1); > } > if (a->curr_state == active || > Hi Jes, is there anything to improve in this patch? I'll submit V4 soon to include some suggestions from Song (in the driver), so if you have suggestions here I could add them to v4. Thanks in advance, Guilherme