All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jes Sorensen <Jes.Sorensen@redhat.com>
To: Gioh Kim <gi-oh.kim@profitbricks.com>
Cc: linux-raid@vger.kernel.org,
	Elmar Gerdes <elmar.gerdes@profitbricks.com>,
	Jinpu Wang <jinpu.wang@profitbricks.com>
Subject: Re: [RFC] super1: error handling for super-block loading
Date: Thu, 12 May 2016 15:43:54 -0400	[thread overview]
Message-ID: <wrfj4ma39i4l.fsf@redhat.com> (raw)
In-Reply-To: <wrfj8tzfb0r0.fsf@redhat.com> (Jes Sorensen's message of "Thu, 12 May 2016 14:16:19 -0400")

Jes Sorensen <Jes.Sorensen@redhat.com> writes:
> Gioh Kim <gi-oh.kim@profitbricks.com> writes:
> 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.

So I started looking at this, and ended up making a bunch of changes
which should both resolve the issue you encountered and also cleans up
this part of the code. I had to change add_internal_bitmap() to return 0
on success, in order to get it cleaned up. Looks like we have a pile of
inconsistencies on the 0 vs 1 as success returns ... guess we won't get
bored.

I just pushed this into git - let me know if it doesn't work for you.

Cheers,
Jes


  reply	other threads:[~2016-05-12 19:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-12 17:38 [RFC] super1: error handling for super-block loading Gioh Kim
2016-05-12 18:16 ` Jes Sorensen
2016-05-12 19:43   ` Jes Sorensen [this message]
2016-05-13 15:15     ` Gioh Kim

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=wrfj4ma39i4l.fsf@redhat.com \
    --to=jes.sorensen@redhat.com \
    --cc=elmar.gerdes@profitbricks.com \
    --cc=gi-oh.kim@profitbricks.com \
    --cc=jinpu.wang@profitbricks.com \
    --cc=linux-raid@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.