Gioh Kim <gi-oh.kim@xxxxxxxxxxxxxxxx> writes:
> I'm not sure yet why mdadm failed to load super-block of disks.
> I checked the kernel log and found I/O error from disks.
> Anyway mdadm needs to handle that error case.
>
> Please review following patch.
>
> ------------------------------------------- 8<
> -------------------------------------------------------------
> From 8cacf56b2d630c7e74bad942779ff7ed5f516d26 Mon Sep 17 00:00:00 2001
> From: Gioh Kim <gi-oh.kim@xxxxxxxxxxxxxxxx>
> Date: Thu, 12 May 2016 19:09:45 +0200
> Subject: [PATCH] super1: error handling for super-block loading
>
> Loading super-block can fail if all sub-devices are faulty
> or have I/O errors.
>
> Signed-off-by: Gioh Kim <gi-oh.kim@xxxxxxxxxxxxxxxx>
> ---
> Grow.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Grow.c b/Grow.c
> index f58c753..fa08522 100755
> --- a/Grow.c
> +++ b/Grow.c
> @@ -389,7 +389,7 @@ int Grow_addbitmap(char *devname, int fd, struct
> context *c, struct shape *s)
> }
> if (strcmp(s->bitmap_file, "internal") == 0 ||
> strcmp(s->bitmap_file, "clustered") == 0) {
> - int rv;
> + int rv = 0;
> int d;
> int offset_setable = 0;
> struct mdinfo *mdi;
> @@ -419,6 +419,7 @@ int Grow_addbitmap(char *devname, int fd, struct
> context *c, struct shape *s)
> if (fd2 < 0)
> continue;
> if (st->ss->load_super(st, fd2, NULL)==0) {
> + rv++;
> if (st->ss->add_internal_bitmap(
> st,
> &s->bitmap_chunk, c->delay, s->write_behind,
> @@ -435,6 +436,10 @@ int Grow_addbitmap(char *devname, int fd, struct
> context *c, struct shape *s)
> close(fd2);
> }
> }
> + if (rv == 0) {
> + pr_err("failed to load super-block.\n");
> + return 1;
> + }
> if (offset_setable) {
> st->ss->getinfo_super(st, mdi, NULL);
> sysfs_init(mdi, fd, NULL);
This is definitly not the right way to solve this problem. Error codes
are negative, and zero should _always_ mean success. Here you suddenly
introduced a new meaning to positive values of rv.
I agree handling the error case needs to be fixed, so a better way to
solve this would be to bail out when the load_super() call fails and
stop there, the same way it does if add_internal_bitmap() fails.
Ie. make it do something like this:
rv = st->ss->load_super(st, fd2, NULL)==0) {
if (!rv) {
if (st->ss->add_internal_bitmap(
....
} else {
pr_err("failed to load super-block.\n");
close(fd2);
return 1;
}
Actually looking at that code, there's a couple of things to do to clean
it up and make it more readable.
Cheers,
Jes
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
