[PATCH v2] xfs_repair: update the manual content about xfs_repair exit status

[PATCH v2] xfs_repair: update the manual content about xfs_repair exit status

[Zorro Lang]

The man 8 xfs_repair said "xfs_repair run without the -n option will
always return a status code of 0". That's not correct.

xfs_repair will return 2 if it finds a fs log which needs to be
replayed or cleared, 1 if runtime error is encountered, and 0 for
all other cases.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

V2 patch did below things:
 - change the description for xfs_repair
 - remove the description for "xfs_repair -L"

Thanks,
Zorro

 man/man8/xfs_repair.8 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/man/man8/xfs_repair.8 b/man/man8/xfs_repair.8
index 1b4d9e3..e45fd90 100644
--- a/man/man8/xfs_repair.8
+++ b/man/man8/xfs_repair.8
@@ -504,12 +504,17 @@ that is known to be free. The entry is therefore invalid and is deleted.
 This message refers to a large directory.
 If the directory were small, the message would read "junking entry ...".
 .SH EXIT STATUS
+.TP
 .B xfs_repair \-n
 (no modify node)
 will return a status of 1 if filesystem corruption was detected and
 0 if no filesystem corruption was detected.
+.TP
 .B xfs_repair
-run without the \-n option will always return a status code of 0.
+run without the \-n option will return a status code of 2 if it finds a
+filesystem log which needs to be replayed(by a mount/umount cycle) or
+cleared(by -L option), 1 if a runtime error is encountered, and 0 in all
+other cases, whether or not filesystem corruption was detected.
 .SH BUGS
 The filesystem to be checked and repaired must have been
 unmounted cleanly using normal system administration procedures
-- 
2.7.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v2] xfs_repair: update the manual content about xfs_repair exit status

[Eric Sandeen]

On 9/13/16 10:54 AM, Zorro Lang wrote:
> The man 8 xfs_repair said "xfs_repair run without the -n option will
> always return a status code of 0". That's not correct.
> 
> xfs_repair will return 2 if it finds a fs log which needs to be
> replayed or cleared, 1 if runtime error is encountered, and 0 for
> all other cases.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> V2 patch did below things:
>  - change the description for xfs_repair
>  - remove the description for "xfs_repair -L"
> 
> Thanks,
> Zorro
> 
>  man/man8/xfs_repair.8 | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/man/man8/xfs_repair.8 b/man/man8/xfs_repair.8
> index 1b4d9e3..e45fd90 100644
> --- a/man/man8/xfs_repair.8
> +++ b/man/man8/xfs_repair.8
> @@ -504,12 +504,17 @@ that is known to be free. The entry is therefore invalid and is deleted.
>  This message refers to a large directory.
>  If the directory were small, the message would read "junking entry ...".
>  .SH EXIT STATUS
> +.TP
>  .B xfs_repair \-n
>  (no modify node)

s/node/mode/, maybe Dave can fix that on commit.  Sorry I missed it the first time.

>  will return a status of 1 if filesystem corruption was detected and
>  0 if no filesystem corruption was detected.
> +.TP
>  .B xfs_repair
> -run without the \-n option will always return a status code of 0.
> +run without the \-n option will return a status code of 2 if it finds a
> +filesystem log which needs to be replayed(by a mount/umount cycle) or

space after replayed

> +cleared(by -L option), 1 if a runtime error is encountered, and 0 in all

space after cleared

> +other cases, whether or not filesystem corruption was detected.

Yep, I think this is ok with the small fixes above.  I hope the "whether or
not" is not more confusing, but I think it probably clarifies.

Dave, if you don't mind the small fixups on the way in,

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Thanks,
-Eric


>  .SH BUGS
>  The filesystem to be checked and repaired must have been
>  unmounted cleanly using normal system administration procedures
> 

Re: [PATCH v2] xfs_repair: update the manual content about xfs_repair exit status

[Darrick J. Wong]

On Tue, Sep 13, 2016 at 11:54:36PM +0800, Zorro Lang wrote:
> The man 8 xfs_repair said "xfs_repair run without the -n option will
> always return a status code of 0". That's not correct.
> 
> xfs_repair will return 2 if it finds a fs log which needs to be
> replayed or cleared, 1 if runtime error is encountered, and 0 for
> all other cases.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> V2 patch did below things:
>  - change the description for xfs_repair
>  - remove the description for "xfs_repair -L"
> 
> Thanks,
> Zorro
> 
>  man/man8/xfs_repair.8 | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/man/man8/xfs_repair.8 b/man/man8/xfs_repair.8
> index 1b4d9e3..e45fd90 100644
> --- a/man/man8/xfs_repair.8
> +++ b/man/man8/xfs_repair.8
> @@ -504,12 +504,17 @@ that is known to be free. The entry is therefore invalid and is deleted.
>  This message refers to a large directory.
>  If the directory were small, the message would read "junking entry ...".
>  .SH EXIT STATUS
> +.TP
>  .B xfs_repair \-n
>  (no modify node)
>  will return a status of 1 if filesystem corruption was detected and
>  0 if no filesystem corruption was detected.
> +.TP
>  .B xfs_repair
> -run without the \-n option will always return a status code of 0.
> +run without the \-n option will return a status code of 2 if it finds a
> +filesystem log which needs to be replayed(by a mount/umount cycle) or
> +cleared(by -L option), 1 if a runtime error is encountered, and 0 in all
> +other cases, whether or not filesystem corruption was detected.

So... I'd rather the documentation about the return code reflect the
status of the filesystem -- 2 means "unclean log, replay it or zap it",
1 means "errors encountered, fs may not be correct", and 0 /should/ mean
"fs is correct".

OTOH I don't know for sure that xfs_repair always cleans up the fs on
the first try.  From my fuzzing experiments a few years ago this seems
to be the case nearly all the time (unlike e2fsck) but not 100%.  ISTR
asking Dave about this, and I think he said that the FS should be clean
if repair returns 0.  But I'll let him reiterate that if it's true;
don't trust my crummy memory, that's why I have filesystems. ;)

--D

>  .SH BUGS
>  The filesystem to be checked and repaired must have been
>  unmounted cleanly using normal system administration procedures
> -- 
> 2.7.4
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v2] xfs_repair: update the manual content about xfs_repair exit status

[Eric Sandeen]

On 9/13/16 11:32 AM, Darrick J. Wong wrote:
> On Tue, Sep 13, 2016 at 11:54:36PM +0800, Zorro Lang wrote:
>> The man 8 xfs_repair said "xfs_repair run without the -n option will
>> always return a status code of 0". That's not correct.
>>
>> xfs_repair will return 2 if it finds a fs log which needs to be
>> replayed or cleared, 1 if runtime error is encountered, and 0 for
>> all other cases.
>>
>> Signed-off-by: Zorro Lang <zlang@redhat.com>
>> ---
>>
>> Hi,
>>
>> V2 patch did below things:
>>  - change the description for xfs_repair
>>  - remove the description for "xfs_repair -L"
>>
>> Thanks,
>> Zorro
>>
>>  man/man8/xfs_repair.8 | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/man/man8/xfs_repair.8 b/man/man8/xfs_repair.8
>> index 1b4d9e3..e45fd90 100644
>> --- a/man/man8/xfs_repair.8
>> +++ b/man/man8/xfs_repair.8
>> @@ -504,12 +504,17 @@ that is known to be free. The entry is therefore invalid and is deleted.
>>  This message refers to a large directory.
>>  If the directory were small, the message would read "junking entry ...".
>>  .SH EXIT STATUS
>> +.TP
>>  .B xfs_repair \-n
>>  (no modify node)
>>  will return a status of 1 if filesystem corruption was detected and
>>  0 if no filesystem corruption was detected.
>> +.TP
>>  .B xfs_repair
>> -run without the \-n option will always return a status code of 0.
>> +run without the \-n option will return a status code of 2 if it finds a
>> +filesystem log which needs to be replayed(by a mount/umount cycle) or
>> +cleared(by -L option), 1 if a runtime error is encountered, and 0 in all
>> +other cases, whether or not filesystem corruption was detected.
> 
> So... I'd rather the documentation about the return code reflect the
> status of the filesystem -- 2 means "unclean log, replay it or zap it",
> 1 means "errors encountered, fs may not be correct", and 0 /should/ mean
> "fs is correct".
> 
> OTOH I don't know for sure that xfs_repair always cleans up the fs on
> the first try.

That's certainly the intent; I can't imagine a manpage documenting
return codes qualified with "... unless bugs happen." :)

>  From my fuzzing experiments a few years ago this seems
> to be the case nearly all the time (unlike e2fsck) but not 100%.

Same here, I fixed what I found...

>  ISTR
> asking Dave about this, and I think he said that the FS should be clean
> if repair returns 0.  But I'll let him reiterate that if it's true;
> don't trust my crummy memory, that's why I have filesystems. ;)

Did you have an alternate wording in mind?

-Eric

> --D
> 
>>  .SH BUGS
>>  The filesystem to be checked and repaired must have been
>>  unmounted cleanly using normal system administration procedures
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v2] xfs_repair: update the manual content about xfs_repair exit status

[Dave Chinner]

On Tue, Sep 13, 2016 at 11:57:59AM -0500, Eric Sandeen wrote:
> On 9/13/16 11:32 AM, Darrick J. Wong wrote:
> > On Tue, Sep 13, 2016 at 11:54:36PM +0800, Zorro Lang wrote:
> >> The man 8 xfs_repair said "xfs_repair run without the -n option will
> >> always return a status code of 0". That's not correct.
> >>
> >> xfs_repair will return 2 if it finds a fs log which needs to be
> >> replayed or cleared, 1 if runtime error is encountered, and 0 for
> >> all other cases.
> >>
> >> Signed-off-by: Zorro Lang <zlang@redhat.com>
> >> ---
> >>
> >> Hi,
> >>
> >> V2 patch did below things:
> >>  - change the description for xfs_repair
> >>  - remove the description for "xfs_repair -L"
> >>
> >> Thanks,
> >> Zorro
> >>
> >>  man/man8/xfs_repair.8 | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/man/man8/xfs_repair.8 b/man/man8/xfs_repair.8
> >> index 1b4d9e3..e45fd90 100644
> >> --- a/man/man8/xfs_repair.8
> >> +++ b/man/man8/xfs_repair.8
> >> @@ -504,12 +504,17 @@ that is known to be free. The entry is therefore invalid and is deleted.
> >>  This message refers to a large directory.
> >>  If the directory were small, the message would read "junking entry ...".
> >>  .SH EXIT STATUS
> >> +.TP
> >>  .B xfs_repair \-n
> >>  (no modify node)
> >>  will return a status of 1 if filesystem corruption was detected and
> >>  0 if no filesystem corruption was detected.
> >> +.TP
> >>  .B xfs_repair
> >> -run without the \-n option will always return a status code of 0.
> >> +run without the \-n option will return a status code of 2 if it finds a
> >> +filesystem log which needs to be replayed(by a mount/umount cycle) or
> >> +cleared(by -L option), 1 if a runtime error is encountered, and 0 in all
> >> +other cases, whether or not filesystem corruption was detected.
> > 
> > So... I'd rather the documentation about the return code reflect the
> > status of the filesystem -- 2 means "unclean log, replay it or zap it",
> > 1 means "errors encountered, fs may not be correct", and 0 /should/ mean
> > "fs is correct".
> > 
> > OTOH I don't know for sure that xfs_repair always cleans up the fs on
> > the first try.
> 
> That's certainly the intent; I can't imagine a manpage documenting
> return codes qualified with "... unless bugs happen." :)

Right - if we hit bugs, all bets are off. But otherwise, the fs
should be repaired and clean after a single pass.

> >  ISTR
> > asking Dave about this, and I think he said that the FS should be clean
> > if repair returns 0.  But I'll let him reiterate that if it's true;
> > don't trust my crummy memory, that's why I have filesystems. ;)
> 
> Did you have an alternate wording in mind?

Yup, 0 = " fs is clean", 1 = "fs is still b0rken",
2 = "couldn't run for whatever reason given"

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v2] xfs_repair: update the manual content about xfs_repair exit status

[Eric Sandeen]

On 9/13/16 4:48 PM, Dave Chinner wrote:
> On Tue, Sep 13, 2016 at 11:57:59AM -0500, Eric Sandeen wrote:
>> On 9/13/16 11:32 AM, Darrick J. Wong wrote:

...

>>> So... I'd rather the documentation about the return code reflect the
>>> status of the filesystem -- 2 means "unclean log, replay it or zap it",
>>> 1 means "errors encountered, fs may not be correct", and 0 /should/ mean
>>> "fs is correct".
>>>
>>> OTOH I don't know for sure that xfs_repair always cleans up the fs on
>>> the first try.
>>
>> That's certainly the intent; I can't imagine a manpage documenting
>> return codes qualified with "... unless bugs happen." :)
> 
> Right - if we hit bugs, all bets are off. But otherwise, the fs
> should be repaired and clean after a single pass.
> 
>>>  ISTR
>>> asking Dave about this, and I think he said that the FS should be clean
>>> if repair returns 0.  But I'll let him reiterate that if it's true;
>>> don't trust my crummy memory, that's why I have filesystems. ;)
>>
>> Did you have an alternate wording in mind?
> 
> Yup, 0 = " fs is clean", 1 = "fs is still b0rken",
> 2 = "couldn't run for whatever reason given"

Technically, 1 = "may or may not be broken" - we really don't know.
We could get an exit of 1 for a consistent filesystem, for example
if some allocation failed... all we know is something bonked out in
the middle.

Maybe "1 == xfs_repair did not run to completion?"

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v2] xfs_repair: update the manual content about xfs_repair exit status

[Dave Chinner]

On Tue, Sep 13, 2016 at 04:52:32PM -0500, Eric Sandeen wrote:
> 
> 
> On 9/13/16 4:48 PM, Dave Chinner wrote:
> > On Tue, Sep 13, 2016 at 11:57:59AM -0500, Eric Sandeen wrote:
> >> On 9/13/16 11:32 AM, Darrick J. Wong wrote:
> 
> ...
> 
> >>> So... I'd rather the documentation about the return code reflect the
> >>> status of the filesystem -- 2 means "unclean log, replay it or zap it",
> >>> 1 means "errors encountered, fs may not be correct", and 0 /should/ mean
> >>> "fs is correct".
> >>>
> >>> OTOH I don't know for sure that xfs_repair always cleans up the fs on
> >>> the first try.
> >>
> >> That's certainly the intent; I can't imagine a manpage documenting
> >> return codes qualified with "... unless bugs happen." :)
> > 
> > Right - if we hit bugs, all bets are off. But otherwise, the fs
> > should be repaired and clean after a single pass.
> > 
> >>>  ISTR
> >>> asking Dave about this, and I think he said that the FS should be clean
> >>> if repair returns 0.  But I'll let him reiterate that if it's true;
> >>> don't trust my crummy memory, that's why I have filesystems. ;)
> >>
> >> Did you have an alternate wording in mind?
> > 
> > Yup, 0 = " fs is clean", 1 = "fs is still b0rken",
> > 2 = "couldn't run for whatever reason given"
> 
> Technically, 1 = "may or may not be broken" - we really don't know.
> We could get an exit of 1 for a consistent filesystem, for example
> if some allocation failed... all we know is something bonked out in
> the middle.
> 
> Maybe "1 == xfs_repair did not run to completion?"

Well, if it fails part way through phase 5, then the filesystem is
most definitely broken, even if it was clean to begin with. i.e.
repair, even when the filesystem is clean, will rebuild parts of the
filesystem from scratch.

And repair nulls out directory entries in phase 4 and doesn't
rebuild those directories till phase 6, so between those points the
filesystem is actually in a corrupt state that requires repair.
hence there is a large scope where a failure in repair really does
mean that we need to run repair again. Hence I think it's simply
safer to explicitly document it as:

	"1 == fs may be even more broken than before repair started,
	so repair needs to be run again"

because "did not run to completion" does not really tell the user
what to do when it occurs.

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs