diff mbox

[1/4] btrfs-progs: adjust the return values for scrub

Message ID 1405564839-28702-1-git-send-email-guihc.fnst@cn.fujitsu.com (mailing list archive)
State Accepted
Headers show

Commit Message

Gui Hecheng July 17, 2014, 2:40 a.m. UTC
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(-)

Comments

Satoru Takeuchi July 30, 2014, 6:03 a.m. UTC | #1
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.

2. It prints messages even if this program runs as foreground mode.
   Checking do_print is necessary.

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.

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
Satoru Takeuchi July 30, 2014, 6:36 a.m. UTC | #2
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.
> 
> 2. It prints messages even if this program runs as foreground mode.
>     Checking do_print is necessary.
> 
> 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

> 
> 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
> 

--
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
Gui Hecheng July 30, 2014, 7:27 a.m. UTC | #3
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
> > 
> 


--
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
diff mbox

Patch

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");
+
 	return 0;
 }