From: "Guilherme G. Piccoli" <gpiccoli@canonical.com>
To: jes.sorensen@gmail.com
Cc: linux-raid@vger.kernel.org, linux-block@vger.kernel.org,
dm-devel@redhat.com, jay.vosburgh@canonical.com,
liu.song.a23@gmail.com, NeilBrown <neilb@suse.com>,
Song Liu <songliubraving@fb.com>
Subject: Re: [PATCH v3 2/2] mdadm: Introduce new array state 'broken' for raid0/linear
Date: Thu, 29 Aug 2019 09:51:38 -0300 [thread overview]
Message-ID: <d8185107-e517-2ce7-7f9b-aa029a97924a@canonical.com> (raw)
In-Reply-To: <20190822161318.26236-2-gpiccoli@canonical.com>
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@suse.com>
> Cc: Song Liu <songliubraving@fb.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
> ---
>
> 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
next prev parent reply other threads:[~2019-08-29 12:51 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-22 16:13 [PATCH v3 1/2] md raid0/linear: Mark array as 'broken' and fail BIOs if a member is gone Guilherme G. Piccoli
2019-08-22 16:13 ` [PATCH v3 2/2] mdadm: Introduce new array state 'broken' for raid0/linear Guilherme G. Piccoli
2019-08-22 21:56 ` Song Liu
2019-08-29 12:51 ` Guilherme G. Piccoli [this message]
2019-08-30 8:17 ` NeilBrown
2019-08-30 12:48 ` Guilherme G. Piccoli
2019-08-22 21:55 ` [PATCH v3 1/2] md raid0/linear: Mark array as 'broken' and fail BIOs if a member is gone Song Liu
2019-08-23 17:48 ` Guilherme G. Piccoli
2019-08-23 17:51 ` Song Liu
2019-08-30 10:47 ` Wols Lists
2019-08-30 11:25 ` Guilherme Piccoli
2019-09-03 19:53 ` Guilherme G. Piccoli
2019-08-30 8:08 ` NeilBrown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d8185107-e517-2ce7-7f9b-aa029a97924a@canonical.com \
--to=gpiccoli@canonical.com \
--cc=dm-devel@redhat.com \
--cc=jay.vosburgh@canonical.com \
--cc=jes.sorensen@gmail.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-raid@vger.kernel.org \
--cc=liu.song.a23@gmail.com \
--cc=neilb@suse.com \
--cc=songliubraving@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).