linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
To: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
Cc: <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH 1/4] btrfs-progs: adjust the return values for scrub
Date: Wed, 30 Jul 2014 15:27:33 +0800	[thread overview]
Message-ID: <1406705253.30821.29.camel@localhost.localdomain> (raw)
In-Reply-To: <53D8927C.6000006@jp.fujitsu.com>

Hi Satoru, I just give a reply to the former 2 mails together here.

On Wed, 2014-07-30 at 15:36 +0900, Satoru Takeuchi wrote:
> Hi Gui,
> 
> (2014/07/30 15:03), Satoru Takeuchi wrote:
> > Hi Gui,
> > 
> > (2014/07/17 11:40), Gui Hecheng wrote:
> >> o Return 0 to indicate success,
> >>     when detected errors were corrected during scrubbing.
> >>     P.s. This is also to facilitate scripting when return value
> >>          is to be checked.
> >> o Warn the users if there are uncorrectable errors detected.
> >>
> >> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
> >> ---
> >>    cmds-scrub.c | 13 ++++++++-----
> >>    1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/cmds-scrub.c b/cmds-scrub.c
> >> index 5265a2b..f9e2b40 100644
> >> --- a/cmds-scrub.c
> >> +++ b/cmds-scrub.c
> >> @@ -1514,14 +1514,17 @@ out:
> >>    	}
> >>    	close_file_or_dir(fdmnt, dirstream);
> >>    
> >> -	if (nothing_to_resume)
> >> -		return 2;
> >>    	if (err)
> >>    		return 1;
> >> -	if (e_correctable)
> >> +	if (nothing_to_resume)
> >> +		return 2;
> >> +	if (e_uncorrectable) {
> >> +		ERR(!do_quiet, "ERROR: There are uncorrectable errors.\n");
> >>    		return 3;
> >> -	if (e_uncorrectable)
> >> -		return 4;
> >> +	}
> >> +	if (e_correctable)
> >> +		ERR(!do_quiet, "WARNING: errors detected during scrubbing, corrected.\n");
> >> +
> > 
> > 1. ERR() messages are not necessary since start command
> >     reports its status in the middle of scrub_start().
> >     It includes both the number of correctable/uncorrectable errors.
	Ok, it may not seems "neccessary", but these ERR()s serve as
conclusions at the end to the users. Because users care whether a scrub
is able to detect errors and if yes, whether the errors are corrected.
So it adds to user experience.

> > 2. It prints messages even if this program runs as foreground mode.
> >     Checking do_print is necessary.
	The do_print serves to control whether "sub procedures(such as
pthread_create/join, scrub_write_progress etc.)" failed or not in
foreground mode, so do_print should not effect conclusion outputs since
all the "sub procedures" succeeded.
 
> > 3. Whether correctable/uncorrectable error exists in a file system
> >     is not related to the exist status of scrub. The role of this
> >     program is
> > 
> >      a) reading all data from all devices and verify checksums, and
> >      b) correct checksum errors "if possible".
> > 
> >     So, I consider scrub only returns error if this program itself
> >     failed to finish.
> 
> Sorry, I understood what you said. It's no problem to exit with
> error codes to detect whether correctable/uncorrectable errors happen
> or not. Sorry for noise.
> 
> Thanks,
> Satoru

Oh, it is really kind of you to spend time reviewing. Thanks very much.

-Gui
> > 
> > Thanks,
> > Satoru
> > 
> >>    	return 0;
> >>    }
> >>    
> >>
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 



      reply	other threads:[~2014-07-30  7:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-17  2:40 [PATCH 1/4] btrfs-progs: adjust the return values for scrub Gui Hecheng
2014-07-17  2:40 ` [PATCH 2/4] btrfs-progs: remove unnecessary judgment for fd in scrub Gui Hecheng
2014-07-30  6:07   ` Satoru Takeuchi
2014-07-17  2:40 ` [PATCH 3/4] btrfs-progs: replace a confusing raw number with a macro Gui Hecheng
2014-07-29 12:16   ` David Sterba
2014-07-29 12:19     ` David Sterba
2014-07-30  1:17       ` Gui Hecheng
2014-07-17  2:40 ` [PATCH 4/4] btrfs-progs: correct manpage option description for scrub Gui Hecheng
2014-07-30  5:20   ` Satoru Takeuchi
2014-07-30  5:39     ` Gui Hecheng
2014-07-30  6:09       ` Satoru Takeuchi
2014-07-30  7:32         ` [PATCH v2] " Gui Hecheng
2014-07-30 10:26           ` Duncan
2014-07-31  1:25             ` Gui Hecheng
2014-07-31 12:24               ` David Sterba
2014-07-30  6:03 ` [PATCH 1/4] btrfs-progs: adjust the return values " Satoru Takeuchi
2014-07-30  6:36   ` Satoru Takeuchi
2014-07-30  7:27     ` Gui Hecheng [this message]

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=1406705253.30821.29.camel@localhost.localdomain \
    --to=guihc.fnst@cn.fujitsu.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=takeuchi_satoru@jp.fujitsu.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).